[PATCH] D51155: [clangd] Allow to merge symbols on-the-fly in global-symbol-builder

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.


================
Comment at: clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp:61
+    /// rely on MR use-case to work properly.
+    llvm::cl::init(false));
+
----------------
ioeric wrote:
> ilya-biryukov wrote:
> > ioeric wrote:
> > > ilya-biryukov wrote:
> > > > ioeric wrote:
> > > > > AFAIK, this flag would basically depend on the executor being used. If executor can provide information like whether all tasks share memory, we can make the decision totally based on the selected executor (i.e. one fewer option).
> > > > That SG. So we could add a virtual method to executor that would provide us with this information and then we can get rid of that option, does that LG?
> > > > However, do we have non-single-binary executor implementations upstream? We need to leave a way to run the code that actually uses YAML serialization to make sure we can experiment when things break.
> > > Sounds good.
> > > 
> > > > However, do we have non-single-binary executor implementations upstream? We need to leave a way to run the code that actually uses YAML serialization to make sure we can experiment when things break.
> > > The framework allows this, so I'm not sure if there is other downstream implementations that do this (probably not?). But I don't think we can get rid of the YAML serialization at this point because we still dump everything to yaml in the end? 
> > I meant the intermediate YAML serialization-deserialization parts.
> > 
> > I'd keep an option to force using the consumer that accumulates YAML symbols, and only allow to use on-the-fly merge when the tool executor reports that it is single-process. Would allow to:
> > - Avoid running on-the-fly merge when the executor does not support it (multi-binary MRs, etc)
> > - Allow to use on-the-fly by default for executors that run in the single binary.
> > - Allow to run with intermediate serialization even in a single binary if `-merge-on-the-fly` is specified.
> > 
> > Does that LG?
> IIUC, there will basically be a flag that allows us to force using yaml intermediate serialization, even when the executor supports on-the-fly merging. By default, we use on-the-fly merging whenever possible. If that's the case, SGTM.
The flag is still called `-merge-on-the-fly`, but it's true by default and can be set to false to reenable intermediate YAML serialization


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51155