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

[Wenlei He via Phabricator via llvm-commits]

wenlei added a comment.

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."

> 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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158689