[PATCH] D41152: Use a custom container to reduce AFDO's memory usage by ~60%

[George Burgess IV via Phabricator via llvm-commits]

george.burgess.iv added a comment.

> Is it enough to simply replace StringMap with std::map<std::string, ..> for small maps? Do you have memory data with that?

Please see the response to your inline comment. The short answer is "that is a good win, but more on the order of 12%." I don't know if that's enough, but can check if you'd like.



================
Comment at: include/llvm/ProfileData/SampleProf.h:188
 
-using BodySampleMap = std::map<LineLocation, SampleRecord>;
-using FunctionSamplesMap = StringMap<FunctionSamples>;
-using CallsiteSampleMap = std::map<LineLocation, FunctionSamplesMap>;
+using BodySampleMap = TinyAssociativeVector<LineLocation, SampleRecord>;
+using FunctionSamplesMap = TinyAssociativeVector<StringRef, FunctionSamples>;
----------------
davidxl wrote:
> I wonder how much is saved by replacing just std::map with TinyAssociativeVector. 
For the 1.5GB profile case:

- Only changing `BodySampleMap` and `CallsiteSampleMap` to `TinyAssociativeVector`s saved us 20% (1.603GB -> 1.288GB)
- Only changing `FunctionSamplesMap` and `CallTargetMap` to `TinyAssociativeVector`s (FunctionSamplesMap using StringRefs) saved us 45% (1.603GB -> 884MB)
- Only changing `FunctionSamplesMap` and `CallTargetMap` to `std::map`s (FunctionSamplesMap using StringRefs) saved us 22% (1.603GB -> 1.219GB)

Changing an individual `StringMap` -> `std::map` or `StringMap`/`std::map` to `TinyAssociativeVector` is always a win in memory usage, in my tests.

The majority of the benefit is from `FunctionSamplesMap`: changing that alone to a `TinyAssociativeVector` drops memory usage by 34% (1.603GB -> 1.025GB). Of that, a reasonable portion of the benefit is from not copying strings around: using a `TinySortedMapVector<std::string, FunctionSamples>` for `FunctionSamplesMap` lands us at 1.159GB of memory use, which is a 26% reduction over a `StringMap`. 

If there are any other combinations you'd like to see, please let me know. I have an earlier version of this that makes toggling these (and any required usage-related changes) really easy. :)


================
Comment at: include/llvm/ProfileData/SampleProf.h:426
 raw_ostream &operator<<(raw_ostream &OS, const FunctionSamples &FS);
-
-/// Sort a LocationT->SampleT map by LocationT.
----------------
davidxl wrote:
> Can you split out this clean up change into a separate patch?
Done. Will submit as a follow-up to this one


https://reviews.llvm.org/D41152