[PATCH] D59941: [DebugInfo] Improve handling of clobbered fragments

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: lib/CodeGen/AsmPrinter/DbgEntityHistoryCalculator.cpp:111
+// pointers.
+using DbgValueEntriesMap = std::map<InlinedEntity, SmallSet<EntryIndex, 1>>;
+
----------------
aprantl wrote:
> dstenb wrote:
> > aprantl wrote:
> > > Do we really need the properties of a std::map here and in the line above?
> > > So far every time I tried using a std::map in AsmPrinter a std::vector + std::sort turned out to be faster and to use less memory.
> > There is at least be no need for iterators to be valid after insertion/deletion, nor is the iteration order important for determinism, which I guess is the two of the main benefits of std::map compared to LLVM's own map implementations. I tried switching both to DenseMap, but I did at least not see any effect on the build time when building the RelWithDebInfo+asan clang binary, but I'm not sure what conclusions I should draw from that.
> > 
> > I will look into switching both to sorted vectors.
> ... or sorted llvm::SmallVectors.
Usually the issue for this tradeoff (map/set (be it standard or LLVM's custom data structures) versus sorted vector (be it std::vector or SmallVector)) is whether or not there's a separate "gather" and "lookup" phase.

If lookups are required while the data structure is also growing, then it's probably inefficient to keep resorting the data structure to keep those lookups efficient.

if there's a defined point where all building is done (& no querying - so you probably know/can guarantee you aren't inserting duplicates without having to test), a single sort and then purely lookup from that point on.

Is that the case here?


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

https://reviews.llvm.org/D59941