This is the following patch of
https://github.com/llvm/llvm-project/pull/66462 to optimize its
performance.
# Motivation
To avoid data races, we choose "per file owns its dependent modules"
model. That said, every TU will own all the required module files so
that we don't need to worry about thread safety. And it looks like we
succeeded that we focus on the interfaces and structure of modules
support in clangd. But after all, this model is not good for
performance. Image we have 10000 TUs import std, we will have 10000
std.pcm in the memory. That is terrible both in time and space.
Given the current modules support in clangd works pretty well (almost
every issue report I received is more or less a clang's issue), I'd like
to improve the performance.
# High Level Changes
After this patch, the built module files will be owned by the module
builder and each TU will only have a reference to the built module
files.
The module builder have a map from module names to built module files.
When a new TU ask for a module file, the module builder will check if
the module file lives in the map and if the module file are up to date.
If yes, the module file will be returned. If no, the module file entry
would be erased in the module builder. We use `shared_ptr<>` to track
module file here so that the other TU owning the out dated module file
won't be affected. The out dated module file will be removed
automatically if other TU gets update or closed.
(I know the out dated module file may not exist due to the `CanReuse`
mechanism. But the design here is natural and can be seen as a redundant
design to make it more robust.)
When we a build a module, we will use the mutex and the condition
variable in the working thread to build it exclusively. All other
threads that also want the module file would have to wait for that
working thread. It might not sounds great but I think if we want to make
it asynchronous, we have to refactor TUScheduler as far as I know.
# Code Structure Changes
Thanks for the previous hard working reviewing, the interfaces almost
don't change in this patch. Almost all the work are isolated in
ModulesBuilder.cpp. A outliner is that we convert `ModulesBuilder` to an
abstract class since the implementation class needs to own the module
files.
And the core function to review is
`ReusableModulesBuilder::getOrBuildModuleFile`. It implements the core
logic to fetch the module file from the cache or build it if the module
file is not in the cache or out of date. And other important entities
are `BuildingModuleMutexes`, `BuildingModuleCVs`, `BuildingModules` and
`ModulesBuildingMutex`. These are mutexes and condition variables to
make sure the thread safety.
# User experience
I've implemented this in our downstream and ask our users to use it. I
also sent it https://github.com/ChuanqiXu9/clangd-for-modules here as
pre-version. The feedbacks are pretty good. And I didn't receive any bug
reports (about the reusable modules builder) yet.
# Other potential improvement
The are other two potential improvements can be done:
1. Scanning cache and a mechanism to get the required module information
more quickly. (Like the module maps in
https://github.com/ChuanqiXu9/clangd-for-modules)
2. Persist the module files. So that after we close the vscode and
reopen it, we can reuse the built module files since the last
invocation.
@kadircet mentioned in
448d8fa880 (diff-fb3ba8a781117ff04736f951a274812cb7ad1678f9d71d4d91870b711ab45da0L285)
that:
> this is definitely a functional change, clangd is used in environments
that solely relies on VFS, and doesn't depend on ASTUnit at all.
> right now this is both introducing a dependency on ASTUnit, and making
all the logical IO physical instead. can you instead use the regular
compiler utilities in clangd, and get the astreader from
CompilerInstance directly, which is VFS-aware, and doesn't depend on
ASTUnit ?
This tries to resolve the problem by creating ASTReader directly and use
VFS to create the FileManager.
It is better to use references instead of pointers as the argument type
of IsModuleFileUpToDate. Since the PrerequisiteModules is always
expected to exist.
This patch extracts ModuleFile class from StandalonePrerequisiteModules
so that we can reuse it further. And also we implement
IsModuleFileUpToDate function to implement
StandalonePrerequisiteModules::CanReuse. Both of them aims to ease the
future improvements to the support of modules in clangd. And both of
them should be NFC.
Alternatives to https://reviews.llvm.org/D153114.
Try to address https://github.com/clangd/clangd/issues/1293.
See the links for design ideas and the consensus so far. We want to have
some initial support in clang18.
This is the initial support for C++20 Modules in clangd.
As suggested by sammccall in https://reviews.llvm.org/D153114,
we should minimize the scope of the initial patch to make it easier
to review and understand so that every one are in the same page:
> Don't attempt any cross-file or cross-version coordination: i.e. don't
> try to reuse BMIs between different files, don't try to reuse BMIs
> between (preamble) reparses of the same file, don't try to persist the
> module graph. Instead, when building a preamble, synchronously scan
> for the module graph, build the required PCMs on the single preamble
> thread with filenames private to that preamble, and then proceed to
> build the preamble.
This patch reflects the above opinions.
# Testing in real-world project
I tested this with a modularized library:
https://github.com/alibaba/async_simple/tree/CXX20Modules. This library
has 3 modules (async_simple, std and asio) and 65 module units. (Note
that a module consists of multiple module units). Both `std` module and
`asio` module have 100k+ lines of code (maybe more, I didn't count). And
async_simple itself has 8k lines of code. This is the scale of the
project.
The result shows that it works pretty well, ..., well, except I need to
wait roughly 10s after opening/editing any file. And this falls in our
expectations. We know it is hard to make it perfect in the first move.
# What this patch does in detail
- Introduced an option `--experimental-modules-support` for the support
for C++20 Modules. So that no matter how bad this is, it wouldn't affect
current users. Following off the page, we'll assume the option is
enabled.
- Introduced two classes `ModuleFilesInfo` and
`ModuleDependencyScanner`. Now `ModuleDependencyScanner` is only used by
`ModuleFilesInfo`.
- The class `ModuleFilesInfo` records the built module files for
specific single source file. The module files can only be built by the
static member function `ModuleFilesInfo::buildModuleFilesInfoFor(PathRef
File, ...)`.
- The class `PreambleData` adds a new member variable with type
`ModuleFilesInfo`. This refers to the needed module files for the
current file. It means the module files info is part of the preamble,
which is suggested in the first patch too.
- In `isPreambleCompatible()`, we add a call to
`ModuleFilesInfo::CanReuse()` to check if the built module files are
still up to date.
- When we build the AST for a source file, we will load the built module
files from ModuleFilesInfo.
# What we need to do next
Let's split the TODOs into clang part and clangd part to make things
more clear.
The TODOs in the clangd part include:
1. Enable reusing module files across source files. The may require us
to bring a ModulesManager like thing which need to handle `scheduling`,
`the possibility of BMI version conflicts` and `various events that can
invalidate the module graph`.
2. Get a more efficient method to get the `<module-name> ->
<module-unit-source>` map. Currently we always scan the whole project
during `ModuleFilesInfo::buildModuleFilesInfoFor(PathRef File, ...)`.
This is clearly inefficient even if the scanning process is pretty fast.
I think the potential solutions include:
- Make a global scanner to monitor the state of every source file like I
did in the first patch. The pain point is that we need to take care of
the data races.
- Ask the build systems to provide the map just like we ask them to
provide the compilation database.
3. Persist the module files. So that we can reuse module files across
clangd invocations or even across clangd instances.
TODOs in the clang part include:
1. Clang should offer an option/mode to skip writing/reading the bodies
of the functions. Or even if we can requrie the parser to skip parsing
the function bodies.
And it looks like we can say the support for C++20 Modules is initially
workable after we made (1) and (2) (or even without (2)).
