[PATCH] D48314: [Frontend] Cache preamble-related data

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

I agree that the optimization is compelling, we do also share preamble in clangd and run code completion in parallel with other actions that need an AST.
However, I would argue that a proper way to introduce the optimization would be to change interface of `ASTUnit` that would somehow allow reusing the PCHs between different `ASTUnit`s.

Having the cache is fine, but I suggest we:

1. Make it explicit in the interface, e.g. `ASTUnit` ctor will accept an extra `PreambleCache` parameter.
2. Have it off by default.

Whenever ASTUnit needs to rebuild a preamble, it can look into the cache and check there's a cached preamble and whether it matches the one that we're building.
After the rebuild is finished, it will try to push the built preamble into the cache.
ASTUnit can store the shared reference to a const preamble, i.e. `std::shared_ptr<const PrecompiledPreamble>`. The cache itself will store the `weak_ptr<const PrecompiledPreamble>`, thus allowing the preambles to be removed when they're not used anymore.

It would allow to:

1. avoid global state in ASTUnit, the cache will have to be created somewhere up the stack and passed to ASTUnit explicitly.
2. avoid complicating ASTUnit with synchronization code, it can be moved to preamble cache.
3. keep changes to the ASTUnit minimal. The only function that needs to be changed is the one building the preamble. Given how complicated ASTUnit is already, I think that's a big win.


https://reviews.llvm.org/D48314