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

[Wenlei He via Phabricator via llvm-commits]

wenlei added a comment.

In D158689#4617548 <https://reviews.llvm.org/D158689#4617548>, @davidxl wrote:

> @wenlei, may I propose having this patch in first (to unblock) @ayermolo while we are discussing longer term strategy?  Another benefit is that we can wait to see if there are other incidence (of using long lifetime reference) can show up so that we have more cases to consider the redesign (if needed).

Back-and-forth reverts/relands are costly and somewhat disruptive, I hope we can gain more confidence before reattempting. Here's my suggestion:

Step 1: use `HashKeyMap<unordered_map, SampleContext, FunctionSamples>` instead of `HashKeyMap<DenseMap, SampleContext, FunctionSamples>` for `SampleProfileMap`. It's a one line change to unblock.

Step 2: a separate patch to attempt DenseMap, coupled with all fixes needed. we can also help test correctness and performance on that change. actually the performance picture may not be that clear. reallocation is expensive, and the fact you didn't run into reallocation in your testing may indicate that the perf benchmark result may also be skewed. We also need to take into account the cost to update references etc.

WDYT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158689