[PATCH] D47063: [clangd] Keep only a limited number of idle ASTs in memory

Ilya Biryukov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Jun 1 03:14:50 PDT 2018


ilya-biryukov added inline comments.


================
Comment at: clangd/TUScheduler.h:66
+              std::chrono::steady_clock::duration UpdateDebounce,
+              ASTRetentionPolicy RetentionPolicy = {});
   ~TUScheduler();
----------------
sammccall wrote:
> ilya-biryukov wrote:
> > sammccall wrote:
> > > does this actually have more than one caller? what's the plan for exposing this option to embedders/CLI users (not saying we necessarily need the latter)?
> > Yes, just one caller outside the tests.
> > The plan was to expose it only in `ClangdServer` for now. Giving this knob in CLI might be useful, if we have good reasons for that, but I hope that we could pick the default that work for everyone instead.
> > Added that as a parameter of `ClangdServer`.
> > 
> > Maybe we should move the default value of 3 to `ClangdServer`? WDYT?
> CLI when needed SG.
> I think we want to give our cloud folks the chance to set it to zero.
> So maybe put the default in ClangdLSPServer? (or make it a param there too and move the value to main)
After an online chat we've decided to keep the default inside the ASTRetentionPolicy struct.
If we add the CLI parameter, we can pass custom values from main to ClangdLSPServer.
And other clients already have a knob to set it in ClangdServer.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D47063





More information about the cfe-commits mailing list