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

[Sam McCall via Phabricator via cfe-commits]

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 nature of a MR is important to its scaling.

As you know, we rely on this code in such a framework :-)
I would be much happier if we could express these constraints/patterns in the open-source code, I think the most promising approach would be to define a more complete clang mapreduce API in Tooling.
But that's way out of scope in this patch. I'm not really sure what to suggest here, happy to talk through it more tomorrow.



================
Comment at: clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp:56
+public:
+  void mergeSymbols(const SymbolSlab &Symbols) {
+    std::lock_guard<std::mutex> Lock(Mut);
----------------
consider calling this `consume`, and the member in SymbolIndexActionFactory etc `Sink`, to emphasize the data flow


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45478