ilya-biryukov abandoned this revision.
ilya-biryukov added a comment.
Herald added subscribers: kadircet, arphaman.
This landed as a different change.
Repository:
rCTE Clang Tools Extra
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D45478/new/
https://reviews.llvm.org/D45478
__
sammccall added a comment.
I think this implementation will have problems in a "real" multi-machine MR
framework. The lifetime of the Merger is the whole program, with output only
coming at the end.
With N workers, each will store >1/N of the symbols (more because of overlap),
and the streaming
ilya-biryukov added inline comments.
Comment at: clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp:53
+/// Combines occurrences of the same symbols across translation units.
+class SymbolMerger {
ilya-biryukov wrote:
> sammccall wrote:
> > Seems reasonab
sammccall added a comment.
In https://reviews.llvm.org/D45478#1071983, @ilya-biryukov wrote:
> In https://reviews.llvm.org/D45478#1064027, @sammccall wrote:
>
> > Is this patch still relevant after haojian's string deduplication?
>
>
> Apparently it does. It has an advantage of distributing the w
ilya-biryukov added a comment.
Herald added a subscriber: jkorous.
In https://reviews.llvm.org/D45478#1064027, @sammccall wrote:
> Is this patch still relevant after haojian's string deduplication?
Apparently it does. It has an advantage of distributing the work more evenly
between the program
sammccall added a comment.
Is this patch still relevant after haojian's string deduplication?
Comment at: clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp:53
+/// Combines occurrences of the same symbols across translation units.
+class SymbolMerger {
hokein added a comment.
Thanks for digging it out!
In upstream, we use `InMemoryToolResults` which saves all duplicated
"std::string"s into the memory, I think we could optimize `InMemoryToolResults`
by using Arena to keep the memory low, I will try it to see whether it can
reduce the memory.
ilya-biryukov created this revision.
ilya-biryukov added reviewers: hokein, sammccall, klimek.
Herald added subscribers: MaskRay, ioeric, jkorous-apple.
This avoids storing intermediate symbols in memory, most of which are
duplicates.
The resulting .yaml file is ~120MB, while intermediate symbols
