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

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.


================
Comment at: clangd/ClangdUnit.h:143
+std::shared_ptr<const PreambleData>
+buildPreamble(PathRef FileName, CompilerInvocation &CI,
+              std::shared_ptr<const PreambleData> OldPreamble,
----------------
sammccall wrote:
> nit: i think filename here is only used for logging, just use Inputs.CompileCommand.Filename?
Tried doing that, but the filename parameter is actually passed to PreambleCallback that updates the dynamic index. Using filename from compile command there seems fragile, so I kept the parameter for now.


================
Comment at: clangd/TUScheduler.h:66
+              std::chrono::steady_clock::duration UpdateDebounce,
+              ASTRetentionPolicy RetentionPolicy = {});
   ~TUScheduler();
----------------
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?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D47063