[PATCH] D158689: [llvm-profdata] Fix dangling reference after D147740

[David Li via Phabricator via llvm-commits]

davidxl added a comment.

In D158689#4617140 <https://reviews.llvm.org/D158689#4617140>, @wenlei wrote:

> In D158689#4616041 <https://reviews.llvm.org/D158689#4616041>, @davidxl wrote:
>
>> I have slightly different opinion on this -- I am on the camp for better performance over a small loss of convenience.
>>
>> This is no different from iterator stability (lack of) for vector type when the loop adds new elements.
>
> It is different. iterators tend to be local and have short lifetime, so it's much more manageable for a reason.
>
> In fact, standard only requires reference stability but not iterator stability for unordered_map.
>
> Quote from standardese:  "If a rehash happens, all iterators are invalidated, but references and pointers to individual elements remain valid."

True for unordered_map, but what I meant is that iterator/reference stability is not always essential to the algorithm -- and we should not be afraid of using containers without such guarantees.

>> We don't disallow use of vector because this. This should be dealt with the right coding idiom (for DenseMap) and proper documutation (on all loops that insert new samples). Once we have good baseline, the maintenance overhead in the future will be small.
>
> It it less about the container itself, but how it is used. Here the container is used to own low level data, with long lifetime, which is why reference stability is useful.

Owning the data is probably not the problem but exposing the pointer/reference to them and keep them for a long life time (in particular, across changes that change the size of the container). How pervasive are those use cases?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158689/new/

https://reviews.llvm.org/D158689