[PATCH] D45478: [clangd] Merge symbols in global-sym-builder on the go

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

[PATCH] D45478: [clangd] Merge symbols in global-sym-builder on the go

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

[PATCH] D45478: [clangd] Merge symbols in global-sym-builder on the go

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

[PATCH] D45478: [clangd] Merge symbols in global-sym-builder on the go

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

[PATCH] D45478: [clangd] Merge symbols in global-sym-builder on the go

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

[PATCH] D45478: [clangd] Merge symbols in global-sym-builder on the go

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 {

[PATCH] D45478: [clangd] Merge symbols in global-sym-builder on the go

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.

[PATCH] D45478: [clangd] Merge symbols in global-sym-builder on the go

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