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

[Wenlei He via Phabricator via llvm-commits]

wenlei added a comment.

In D158689#4615531 <https://reviews.llvm.org/D158689#4615531>, @huangjd wrote:

> DenseMap is significantly faster than std::unordered_map as previously used.
>
> Benchmarking with an internal profile for production, when reading the function offset table alone, the original implementation (before D147740 <https://reviews.llvm.org/D147740>), DenseMap, and unordered_map takes 1.2, 0.78, 0.82 s respectively, and reading the full profiles takes 34.6, 29.0, 31.4 s respectively. That means using a std::unordered_map is 5-8% slower. (Meanwhile the refactoring itself has been very significant regardless of which container to use)

Regardless of how fast DenseMap is, correctness and robustness are higher priority. Evidently the change breaks reference stability used with the implementation and has caused a lot of breakages. I'm not confident this is the last fixed needed.

I'm fine with `HashKeyMap<unordered_map, SampleContext, FunctionSamples>`, but I see not providing reference stability as fragile. Spraying the code with pointer/reference updates here and there is not only fragile but also hacky IMO.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158689