[all-commits] [llvm/llvm-project] e385e0: [clangd] [Modules] Support Reusable Modules Builde...

[Chuanqi Xu via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: e385e0d3e71e17da0b2023f480259c95923707bd
      https://github.com/llvm/llvm-project/commit/e385e0d3e71e17da0b2023f480259c95923707bd
  Author: Chuanqi Xu <yedeng.yd at linux.alibaba.com>
  Date:   2024-11-12 (Tue, 12 Nov 2024)

  Changed paths:
    M clang-tools-extra/clangd/ModulesBuilder.cpp
    M clang-tools-extra/clangd/ModulesBuilder.h
    M clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp

  Log Message:
  -----------
  [clangd] [Modules] Support Reusable Modules Builder (#106683)

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.



To unsubscribe from these emails, change your notification settings at https://github.com/llvm/llvm-project/settings/notifications