[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
Arthapz wrote: i tried it for a while, it work very well ! thx u https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
@@ -316,36 +295,187 @@ llvm::Error buildModuleFile(llvm::StringRef ModuleName, if (Clang->getDiagnostics().hasErrorOccurred()) return llvm::createStringError("Compilation failed"); - BuiltModuleFiles.addModuleFile(ModuleName, Inputs.CompileCommand.Output); - return llvm::Error::success(); + return ModuleFile{ModuleName, Inputs.CompileCommand.Output}; } + +bool ReusablePrerequisiteModules::canReuse( +const CompilerInvocation &CI, +llvm::IntrusiveRefCntPtr VFS) const { + if (RequiredModules.empty()) +return true; + + SmallVector BMIPaths; + for (auto &MF : RequiredModules) +BMIPaths.push_back(MF->getModuleFilePath()); + return IsModuleFilesUpToDate(BMIPaths, *this, VFS); +} + +class ModuleFileCache { +public: + ModuleFileCache(const GlobalCompilationDatabase &CDB) : CDB(CDB) {} + const GlobalCompilationDatabase &getCDB() const { return CDB; } + + std::shared_ptr getModule(StringRef ModuleName); + + void add(StringRef ModuleName, std::shared_ptr ModuleFile) { +std::lock_guard Lock(ModuleFilesMutex); + +ModuleFiles.insert_or_assign(ModuleName, ModuleFile); + } + + void remove(StringRef ModuleName); + +private: + const GlobalCompilationDatabase &CDB; + + llvm::StringMap> ModuleFiles; + // Mutex to guard accesses to ModuleFiles. + std::mutex ModuleFilesMutex; +}; + +std::shared_ptr +ModuleFileCache::getModule(StringRef ModuleName) { + std::lock_guard Lock(ModuleFilesMutex); + + auto Iter = ModuleFiles.find(ModuleName); + if (Iter == ModuleFiles.end()) +return nullptr; + + if (Iter->second.expired()) { +ModuleFiles.erase(Iter); +return nullptr; + } + + return Iter->second.lock(); +} + +void ModuleFileCache::remove(StringRef ModuleName) { + std::lock_guard Lock(ModuleFilesMutex); + + auto Iter = ModuleFiles.find(ModuleName); + if (Iter == ModuleFiles.end()) +return; + + ModuleFiles.erase(Iter); +} + +/// Collect the directly and indirectly required module names for \param +/// ModuleName in topological order. The \param ModuleName is guaranteed to +/// be the last element in \param ModuleNames. +llvm::SmallVector getAllRequiredModules(ProjectModules &MDB, + StringRef ModuleName) { + llvm::SmallVector ModuleNames; + llvm::StringSet<> ModuleNamesSet; + + auto traversal = [&](StringRef ModuleName, auto recursionHelper) -> void { +ModuleNamesSet.insert(ModuleName); + +for (StringRef RequiredModuleName : + MDB.getRequiredModules(MDB.getSourceForModuleName(ModuleName))) + if (ModuleNamesSet.insert(RequiredModuleName).second) +recursionHelper(RequiredModuleName, recursionHelper); + +ModuleNames.push_back(ModuleName); + }; + traversal(ModuleName, traversal); + + return ModuleNames; +} + } // namespace +class ModulesBuilder::ModulesBuilderImpl { +public: + ModulesBuilderImpl(const GlobalCompilationDatabase &CDB) : Cache(CDB) {} + + const GlobalCompilationDatabase &getCDB() const { return Cache.getCDB(); } + + llvm::Error + getOrBuildModuleFile(StringRef ModuleName, const ThreadsafeFS &TFS, + ProjectModules &MDB, + ReusablePrerequisiteModules &BuiltModuleFiles); + +private: + ModuleFileCache Cache; +}; + +llvm::Error ModulesBuilder::ModulesBuilderImpl::getOrBuildModuleFile( +StringRef ModuleName, const ThreadsafeFS &TFS, ProjectModules &MDB, +ReusablePrerequisiteModules &BuiltModuleFiles) { + if (BuiltModuleFiles.isModuleUnitBuilt(ModuleName)) +return llvm::Error::success(); + + PathRef ModuleUnitFileName = MDB.getSourceForModuleName(ModuleName); + /// It is possible that we're meeting third party modules (modules whose + /// source are not in the project. e.g, the std module may be a third-party + /// module for most project) or something wrong with the implementation of + /// ProjectModules. + /// FIXME: How should we treat third party modules here? If we want to ignore + /// third party modules, we should return true instead of false here. + /// Currently we simply bail out. + if (ModuleUnitFileName.empty()) +return llvm::createStringError( +llvm::formatv("Don't get the module unit for module {0}", ModuleName)); + + // Get Required modules in topological order. + auto ReqModuleNames = getAllRequiredModules(MDB, ModuleName); + for (llvm::StringRef ReqModuleName : ReqModuleNames) { +if (BuiltModuleFiles.isModuleUnitBuilt(ModuleName)) + continue; + +if (auto Cached = Cache.getModule(ReqModuleName)) { + if (IsModuleFileUpToDate(Cached->getModuleFilePath(), BuiltModuleFiles, + TFS.view(std::nullopt))) { +log("Reusing module {0} from {1}", ModuleName, +Cached->getModuleFilePath()); +BuiltModuleFiles.addModuleFile(std::move(Cached)); +continue; + } + Cache.remove(ReqModuleName); +} + +llvm::SmallString<256> ModuleFilesPrefix = +getUniqueModuleFilesPa
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
llvm-ci wrote: LLVM Buildbot has detected a new failure on builder `libc-x86_64-debian-fullbuild-dbg-asan` running on `libc-x86_64-debian-fullbuild` while building `clang-tools-extra` at step 4 "annotate". Full details are available at: https://lab.llvm.org/buildbot/#/builders/171/builds/10157 Here is the relevant piece of the build log for the reference ``` Step 4 (annotate) failure: 'python ../llvm-zorg/zorg/buildbot/builders/annotated/libc-linux.py ...' (failure) ... [ OK ] LlvmLibcRoundToIntegerTest.SubnormalRange (944 ms) Ran 3 tests. PASS: 3 FAIL: 0 [962/1095] Running unit test libc.test.src.sys.prctl.linux.prctl_test [==] Running 2 tests from 1 test suite. [ RUN ] LlvmLibcSysPrctlTest.GetSetName [ OK ] LlvmLibcSysPrctlTest.GetSetName (34 us) [ RUN ] LlvmLibcSysPrctlTest.GetTHPDisable [ OK ] LlvmLibcSysPrctlTest.GetTHPDisable (8 us) Ran 2 tests. PASS: 2 FAIL: 0 [963/1095] Running unit test libc.test.src.sys.statvfs.linux.statvfs_test FAILED: projects/libc/test/src/sys/statvfs/linux/CMakeFiles/libc.test.src.sys.statvfs.linux.statvfs_test /home/llvm-libc-buildbot/buildbot-worker/libc-x86_64-debian-fullbuild/libc-x86_64-debian-fullbuild-dbg-asan/build/projects/libc/test/src/sys/statvfs/linux/CMakeFiles/libc.test.src.sys.statvfs.linux.statvfs_test cd /home/llvm-libc-buildbot/buildbot-worker/libc-x86_64-debian-fullbuild/libc-x86_64-debian-fullbuild-dbg-asan/build/projects/libc/test/src/sys/statvfs/linux && /home/llvm-libc-buildbot/buildbot-worker/libc-x86_64-debian-fullbuild/libc-x86_64-debian-fullbuild-dbg-asan/build/projects/libc/test/src/sys/statvfs/linux/libc.test.src.sys.statvfs.linux.statvfs_test.__build__ [==] Running 2 tests from 1 test suite. [ RUN ] LlvmLibcSysStatvfsTest.StatvfsBasic [ OK ] LlvmLibcSysStatvfsTest.StatvfsBasic (14 us) [ RUN ] LlvmLibcSysStatvfsTest.StatvfsInvalidPath /home/llvm-libc-buildbot/buildbot-worker/libc-x86_64-debian-fullbuild/libc-x86_64-debian-fullbuild-dbg-asan/llvm-project/libc/test/src/sys/statvfs/linux/statvfs_test.cpp:37: FAILURE Failed to match LIBC_NAMESPACE::mkdirat(AT_FDCWD, TEST_DIR, S_IRWXU) against Succeeds(0). Expected return value to be equal to 0 but got -1. Expected errno to be equal to "Success" but got "File exists". [ FAILED ] LlvmLibcSysStatvfsTest.StatvfsInvalidPath Ran 2 tests. PASS: 1 FAIL: 1 [964/1095] Running unit test libc.test.src.sys.auxv.linux.getauxval_test [==] Running 1 test from 1 test suite. [ RUN ] LlvmLibcGetauxvalTest.Basic [ OK ] LlvmLibcGetauxvalTest.Basic (117 us) Ran 1 tests. PASS: 1 FAIL: 0 [965/1095] Running unit test libc.test.src.sys.epoll.linux.epoll_create1_test [==] Running 2 tests from 1 test suite. [ RUN ] LlvmLibcEpollCreate1Test.Basic [ OK ] LlvmLibcEpollCreate1Test.Basic (53 us) [ RUN ] LlvmLibcEpollCreate1Test.CloseOnExecute [ OK ] LlvmLibcEpollCreate1Test.CloseOnExecute (11 us) Ran 2 tests. PASS: 2 FAIL: 0 [966/1095] Running unit test libc.test.src.unistd.access_test [==] Running 2 tests from 1 test suite. [ RUN ] LlvmLibcAccessTest.CreateAndTest [ OK ] LlvmLibcAccessTest.CreateAndTest (194 us) [ RUN ] LlvmLibcAccessTest.AccessNonExistentFile [ OK ] LlvmLibcAccessTest.AccessNonExistentFile (8 us) Ran 2 tests. PASS: 2 FAIL: 0 [967/1095] Running unit test libc.test.src.sys.statvfs.linux.fstatvfs_test [==] Running 2 tests from 1 test suite. [ RUN ] LlvmLibcSysFStatvfsTest.FStatvfsBasic [ OK ] LlvmLibcSysFStatvfsTest.FStatvfsBasic (39 us) [ RUN ] LlvmLibcSysFStatvfsTest.FStatvfsInvalidPath [ OK ] LlvmLibcSysFStatvfsTest.FStatvfsInvalidPath (259 us) Ran 2 tests. PASS: 2 FAIL: 0 [968/1095] Running unit test libc.test.src.sys.wait.wait4_test Step 8 (libc-unit-tests) failure: libc-unit-tests (failure) ... [ OK ] LlvmLibcRoundToIntegerTest.SubnormalRange (944 ms) Ran 3 tests. PASS: 3 FAIL: 0 [962/1095] Running unit test libc.test.src.sys.prctl.linux.prctl_test [==] Running 2 tests from 1 test suite. [ RUN ] LlvmLibcSysPrctlTest.GetSetName [ OK ] LlvmLibcSysPrctlTest.GetSetName (34 us) [ RUN ] LlvmLibcSysPrctlTest.GetTHPDisable [ OK ] LlvmLibcSysPrctlTest.GetTHPDisable (8 us) Ran 2 tests. PASS: 2 FAIL: 0 [963/1095] Running unit test libc.test.src.sys.statvfs.linux.statvfs_test FAILED: projects/libc/test/src/sys/statvfs/linux/CMakeFiles/libc.test.src.sys.statvfs.linux.statvfs_test /home/llvm-libc-buildbot/buildbot-worker/libc-x86_64-debian-fullbuild/libc-x86_64-debian-fullbuild-dbg-asan/build/projects/libc/test/src/sys/statvfs/linux/CMakeFiles/libc.test.src.sys.statvfs.linux.statvfs_test cd /home/llvm-libc-buildbot/buildbot-worker/libc-x86_64-debian-fullbuild/libc-x86_64-debian-fullbuild-dbg-asan/build/projects/libc/test/src/sys/statvfs/linux && /home/llvm-libc-buildbot/buildbot-worker/libc-x86_64-debian-fullbuild/libc-x86_64-debian-fullbuild-dbg-asan/build/projects/libc/test
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
llvm-ci wrote: LLVM Buildbot has detected a new failure on builder `libc-aarch64-ubuntu-fullbuild-dbg` running on `libc-aarch64-ubuntu` while building `clang-tools-extra` at step 4 "annotate". Full details are available at: https://lab.llvm.org/buildbot/#/builders/71/builds/10350 Here is the relevant piece of the build log for the reference ``` Step 4 (annotate) failure: 'python ../llvm-zorg/zorg/buildbot/builders/annotated/libc-linux.py ...' (failure) ... [ OK ] LlvmLibcStatTest.NonExistentFile (3 us) Ran 2 tests. PASS: 2 FAIL: 0 [757/881] Running unit test libc.test.src.sys.stat.fstat_test [==] Running 2 tests from 1 test suite. [ RUN ] LlvmLibcFStatTest.CreatAndReadMode [ OK ] LlvmLibcFStatTest.CreatAndReadMode (69 us) [ RUN ] LlvmLibcFStatTest.NonExistentFile [ OK ] LlvmLibcFStatTest.NonExistentFile (2 us) Ran 2 tests. PASS: 2 FAIL: 0 [758/881] Running unit test libc.test.src.sys.statvfs.linux.statvfs_test FAILED: projects/libc/test/src/sys/statvfs/linux/CMakeFiles/libc.test.src.sys.statvfs.linux.statvfs_test cd /home/libc-buildbot/libc-aarch64-ubuntu/libc-aarch64-ubuntu-fullbuild-dbg/build/projects/libc/test/src/sys/statvfs/linux && /home/libc-buildbot/libc-aarch64-ubuntu/libc-aarch64-ubuntu-fullbuild-dbg/build/projects/libc/test/src/sys/statvfs/linux/libc.test.src.sys.statvfs.linux.statvfs_test.__build__ [==] Running 2 tests from 1 test suite. [ RUN ] LlvmLibcSysStatvfsTest.StatvfsBasic [ OK ] LlvmLibcSysStatvfsTest.StatvfsBasic (10 us) [ RUN ] LlvmLibcSysStatvfsTest.StatvfsInvalidPath /home/libc-buildbot/libc-aarch64-ubuntu/libc-aarch64-ubuntu-fullbuild-dbg/llvm-project/libc/test/src/sys/statvfs/linux/statvfs_test.cpp:37: FAILURE Failed to match LIBC_NAMESPACE::mkdirat(AT_FDCWD, TEST_DIR, S_IRWXU) against Succeeds(0). Expected return value to be equal to 0 but got -1. Expected errno to be equal to "Success" but got "File exists". [ FAILED ] LlvmLibcSysStatvfsTest.StatvfsInvalidPath Ran 2 tests. PASS: 1 FAIL: 1 [759/881] Running unit test libc.test.src.sys.stat.lstat_test [==] Running 2 tests from 1 test suite. [ RUN ] LlvmLibcLStatTest.CreatAndReadMode [ OK ] LlvmLibcLStatTest.CreatAndReadMode (57 us) [ RUN ] LlvmLibcLStatTest.NonExistentFile [ OK ] LlvmLibcLStatTest.NonExistentFile (3 us) Ran 2 tests. PASS: 2 FAIL: 0 [760/881] Running unit test libc.test.src.sys.statvfs.linux.fstatvfs_test [==] Running 2 tests from 1 test suite. [ RUN ] LlvmLibcSysFStatvfsTest.FStatvfsBasic [ OK ] LlvmLibcSysFStatvfsTest.FStatvfsBasic (13 us) [ RUN ] LlvmLibcSysFStatvfsTest.FStatvfsInvalidPath [ OK ] LlvmLibcSysFStatvfsTest.FStatvfsInvalidPath (78 us) Ran 2 tests. PASS: 2 FAIL: 0 [761/881] Running unit test libc.test.src.string.bcopy_test.__unit__ [==] Running 7 tests from 1 test suite. [ RUN ] LlvmLibcBcopyTest.MoveZeroByte [ OK ] LlvmLibcBcopyTest.MoveZeroByte (1 us) [ RUN ] LlvmLibcBcopyTest.DstAndSrcPointToSameAddress [ OK ] LlvmLibcBcopyTest.DstAndSrcPointToSameAddress (1 us) [ RUN ] LlvmLibcBcopyTest.DstStartsBeforeSrc [ OK ] LlvmLibcBcopyTest.DstStartsBeforeSrc (0 ns) [ RUN ] LlvmLibcBcopyTest.DstStartsAfterSrc [ OK ] LlvmLibcBcopyTest.DstStartsAfterSrc (1 us) [ RUN ] LlvmLibcBcopyTest.SrcFollowDst [ OK ] LlvmLibcBcopyTest.SrcFollowDst (1 us) [ RUN ] LlvmLibcBcopyTest.DstFollowSrc Step 8 (libc-unit-tests) failure: libc-unit-tests (failure) ... [ OK ] LlvmLibcStatTest.NonExistentFile (3 us) Ran 2 tests. PASS: 2 FAIL: 0 [757/881] Running unit test libc.test.src.sys.stat.fstat_test [==] Running 2 tests from 1 test suite. [ RUN ] LlvmLibcFStatTest.CreatAndReadMode [ OK ] LlvmLibcFStatTest.CreatAndReadMode (69 us) [ RUN ] LlvmLibcFStatTest.NonExistentFile [ OK ] LlvmLibcFStatTest.NonExistentFile (2 us) Ran 2 tests. PASS: 2 FAIL: 0 [758/881] Running unit test libc.test.src.sys.statvfs.linux.statvfs_test FAILED: projects/libc/test/src/sys/statvfs/linux/CMakeFiles/libc.test.src.sys.statvfs.linux.statvfs_test cd /home/libc-buildbot/libc-aarch64-ubuntu/libc-aarch64-ubuntu-fullbuild-dbg/build/projects/libc/test/src/sys/statvfs/linux && /home/libc-buildbot/libc-aarch64-ubuntu/libc-aarch64-ubuntu-fullbuild-dbg/build/projects/libc/test/src/sys/statvfs/linux/libc.test.src.sys.statvfs.linux.statvfs_test.__build__ [==] Running 2 tests from 1 test suite. [ RUN ] LlvmLibcSysStatvfsTest.StatvfsBasic [ OK ] LlvmLibcSysStatvfsTest.StatvfsBasic (10 us) [ RUN ] LlvmLibcSysStatvfsTest.StatvfsInvalidPath /home/libc-buildbot/libc-aarch64-ubuntu/libc-aarch64-ubuntu-fullbuild-dbg/llvm-project/libc/test/src/sys/statvfs/linux/statvfs_test.cpp:37: FAILURE Failed to match LIBC_NAMESPACE::mkdirat(AT_FDCWD, TEST_DIR, S_IRWXU) against Succeeds(0). Expected return value to be equal to 0 but got -1. Expected errno to be equal to "Success" but got "File exists".
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
https://github.com/ChuanqiXu9 closed https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
ChuanqiXu9 wrote: Thank you very much ! https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
@@ -316,36 +295,187 @@ llvm::Error buildModuleFile(llvm::StringRef ModuleName, if (Clang->getDiagnostics().hasErrorOccurred()) return llvm::createStringError("Compilation failed"); - BuiltModuleFiles.addModuleFile(ModuleName, Inputs.CompileCommand.Output); - return llvm::Error::success(); + return ModuleFile{ModuleName, Inputs.CompileCommand.Output}; } + +bool ReusablePrerequisiteModules::canReuse( +const CompilerInvocation &CI, +llvm::IntrusiveRefCntPtr VFS) const { + if (RequiredModules.empty()) +return true; + + SmallVector BMIPaths; + for (auto &MF : RequiredModules) +BMIPaths.push_back(MF->getModuleFilePath()); + return IsModuleFilesUpToDate(BMIPaths, *this, VFS); +} + +class ModuleFileCache { +public: + ModuleFileCache(const GlobalCompilationDatabase &CDB) : CDB(CDB) {} + const GlobalCompilationDatabase &getCDB() const { return CDB; } + + std::shared_ptr getModule(StringRef ModuleName); + + void add(StringRef ModuleName, std::shared_ptr ModuleFile) { +std::lock_guard Lock(ModuleFilesMutex); + +ModuleFiles.insert_or_assign(ModuleName, ModuleFile); + } + + void remove(StringRef ModuleName); + +private: + const GlobalCompilationDatabase &CDB; + + llvm::StringMap> ModuleFiles; + // Mutex to guard accesses to ModuleFiles. + std::mutex ModuleFilesMutex; +}; + +std::shared_ptr +ModuleFileCache::getModule(StringRef ModuleName) { + std::lock_guard Lock(ModuleFilesMutex); + + auto Iter = ModuleFiles.find(ModuleName); + if (Iter == ModuleFiles.end()) +return nullptr; + + if (Iter->second.expired()) { +ModuleFiles.erase(Iter); +return nullptr; + } + + return Iter->second.lock(); +} + +void ModuleFileCache::remove(StringRef ModuleName) { + std::lock_guard Lock(ModuleFilesMutex); + + auto Iter = ModuleFiles.find(ModuleName); + if (Iter == ModuleFiles.end()) +return; + + ModuleFiles.erase(Iter); +} + +/// Collect the directly and indirectly required module names for \param +/// ModuleName in topological order. The \param ModuleName is guaranteed to +/// be the last element in \param ModuleNames. +llvm::SmallVector getAllRequiredModules(ProjectModules &MDB, + StringRef ModuleName) { + llvm::SmallVector ModuleNames; + llvm::StringSet<> ModuleNamesSet; + + auto traversal = [&](StringRef ModuleName, auto recursionHelper) -> void { +ModuleNamesSet.insert(ModuleName); + +for (StringRef RequiredModuleName : + MDB.getRequiredModules(MDB.getSourceForModuleName(ModuleName))) + if (ModuleNamesSet.insert(RequiredModuleName).second) +recursionHelper(RequiredModuleName, recursionHelper); + +ModuleNames.push_back(ModuleName); + }; + traversal(ModuleName, traversal); + + return ModuleNames; +} + } // namespace +class ModulesBuilder::ModulesBuilderImpl { +public: + ModulesBuilderImpl(const GlobalCompilationDatabase &CDB) : Cache(CDB) {} + + const GlobalCompilationDatabase &getCDB() const { return Cache.getCDB(); } + + llvm::Error + getOrBuildModuleFile(StringRef ModuleName, const ThreadsafeFS &TFS, + ProjectModules &MDB, + ReusablePrerequisiteModules &BuiltModuleFiles); + +private: + ModuleFileCache Cache; +}; + +llvm::Error ModulesBuilder::ModulesBuilderImpl::getOrBuildModuleFile( +StringRef ModuleName, const ThreadsafeFS &TFS, ProjectModules &MDB, +ReusablePrerequisiteModules &BuiltModuleFiles) { + if (BuiltModuleFiles.isModuleUnitBuilt(ModuleName)) +return llvm::Error::success(); + + PathRef ModuleUnitFileName = MDB.getSourceForModuleName(ModuleName); + /// It is possible that we're meeting third party modules (modules whose + /// source are not in the project. e.g, the std module may be a third-party + /// module for most project) or something wrong with the implementation of + /// ProjectModules. + /// FIXME: How should we treat third party modules here? If we want to ignore + /// third party modules, we should return true instead of false here. + /// Currently we simply bail out. + if (ModuleUnitFileName.empty()) +return llvm::createStringError( +llvm::formatv("Don't get the module unit for module {0}", ModuleName)); + + // Get Required modules in topological order. + auto ReqModuleNames = getAllRequiredModules(MDB, ModuleName); + for (llvm::StringRef ReqModuleName : ReqModuleNames) { +if (BuiltModuleFiles.isModuleUnitBuilt(ModuleName)) + continue; + +if (auto Cached = Cache.getModule(ReqModuleName)) { + if (IsModuleFileUpToDate(Cached->getModuleFilePath(), BuiltModuleFiles, + TFS.view(std::nullopt))) { +log("Reusing module {0} from {1}", ModuleName, +Cached->getModuleFilePath()); +BuiltModuleFiles.addModuleFile(std::move(Cached)); +continue; + } + Cache.remove(ReqModuleName); +} + +llvm::SmallString<256> ModuleFilesPrefix = +getUniqueModuleFilesPa
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
https://github.com/ChuanqiXu9 edited https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
https://github.com/kadircet approved this pull request. thanks a lot for bearing with me! LGTM, let's ship it! https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
https://github.com/ChuanqiXu9 updated https://github.com/llvm/llvm-project/pull/106683 >From 15aa6af1f5d22e4b837e8e2fd49469310ffbe7f1 Mon Sep 17 00:00:00 2001 From: Chuanqi Xu Date: Fri, 30 Aug 2024 15:11:07 +0800 Subject: [PATCH 1/7] [clangd] [Modules] Support Reusable Modules Builder --- clang-tools-extra/clangd/ModulesBuilder.cpp | 375 ++ clang-tools-extra/clangd/ModulesBuilder.h | 8 +- .../unittests/PrerequisiteModulesTest.cpp | 36 ++ 3 files changed, 344 insertions(+), 75 deletions(-) diff --git a/clang-tools-extra/clangd/ModulesBuilder.cpp b/clang-tools-extra/clangd/ModulesBuilder.cpp index 97f67ddf5495a2..c4fb3d4010d370 100644 --- a/clang-tools-extra/clangd/ModulesBuilder.cpp +++ b/clang-tools-extra/clangd/ModulesBuilder.cpp @@ -13,6 +13,7 @@ #include "clang/Frontend/FrontendActions.h" #include "clang/Serialization/ASTReader.h" #include "clang/Serialization/InMemoryModuleCache.h" +#include "llvm/ADT/ScopeExit.h" namespace clang { namespace clangd { @@ -192,33 +193,27 @@ bool IsModuleFilesUpToDate( }); } -// StandalonePrerequisiteModules - stands for PrerequisiteModules for which all +// ReusablePrerequisiteModules - stands for PrerequisiteModules for which all // the required modules are built successfully. All the module files -// are owned by the StandalonePrerequisiteModules class. -// -// Any of the built module files won't be shared with other instances of the -// class. So that we can avoid worrying thread safety. -// -// We don't need to worry about duplicated module names here since the standard -// guarantees the module names should be unique to a program. -class StandalonePrerequisiteModules : public PrerequisiteModules { +// are owned by the modules builder. +class ReusablePrerequisiteModules : public PrerequisiteModules { public: - StandalonePrerequisiteModules() = default; + ReusablePrerequisiteModules() = default; - StandalonePrerequisiteModules(const StandalonePrerequisiteModules &) = delete; - StandalonePrerequisiteModules - operator=(const StandalonePrerequisiteModules &) = delete; - StandalonePrerequisiteModules(StandalonePrerequisiteModules &&) = delete; - StandalonePrerequisiteModules - operator=(StandalonePrerequisiteModules &&) = delete; + ReusablePrerequisiteModules(const ReusablePrerequisiteModules &) = delete; + ReusablePrerequisiteModules + operator=(const ReusablePrerequisiteModules &) = delete; + ReusablePrerequisiteModules(ReusablePrerequisiteModules &&) = delete; + ReusablePrerequisiteModules + operator=(ReusablePrerequisiteModules &&) = delete; - ~StandalonePrerequisiteModules() override = default; + ~ReusablePrerequisiteModules() override = default; void adjustHeaderSearchOptions(HeaderSearchOptions &Options) const override { // Appending all built module files. for (auto &RequiredModule : RequiredModules) Options.PrebuiltModuleFiles.insert_or_assign( - RequiredModule.ModuleName, RequiredModule.ModuleFilePath); + RequiredModule->ModuleName, RequiredModule->ModuleFilePath); } bool canReuse(const CompilerInvocation &CI, @@ -228,54 +223,30 @@ class StandalonePrerequisiteModules : public PrerequisiteModules { return BuiltModuleNames.contains(ModuleName); } - void addModuleFile(llvm::StringRef ModuleName, - llvm::StringRef ModuleFilePath) { -RequiredModules.emplace_back(ModuleName, ModuleFilePath); -BuiltModuleNames.insert(ModuleName); + void addModuleFile(std::shared_ptr BMI) { +BuiltModuleNames.insert(BMI->ModuleName); +RequiredModules.emplace_back(std::move(BMI)); } private: - llvm::SmallVector RequiredModules; + mutable llvm::SmallVector, 8> RequiredModules; // A helper class to speedup the query if a module is built. llvm::StringSet<> BuiltModuleNames; }; -// Build a module file for module with `ModuleName`. The information of built -// module file are stored in \param BuiltModuleFiles. -llvm::Error buildModuleFile(llvm::StringRef ModuleName, -const GlobalCompilationDatabase &CDB, -const ThreadsafeFS &TFS, ProjectModules &MDB, -PathRef ModuleFilesPrefix, -StandalonePrerequisiteModules &BuiltModuleFiles) { - if (BuiltModuleFiles.isModuleUnitBuilt(ModuleName)) -return llvm::Error::success(); - - PathRef ModuleUnitFileName = MDB.getSourceForModuleName(ModuleName); - // It is possible that we're meeting third party modules (modules whose - // source are not in the project. e.g, the std module may be a third-party - // module for most projects) or something wrong with the implementation of - // ProjectModules. - // FIXME: How should we treat third party modules here? If we want to ignore - // third party modules, we should return true instead of false here. - // Currently we simply bail out. - if (ModuleUnitFileName.empty()) -return llvm::createStringErro
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
@@ -316,36 +295,187 @@ llvm::Error buildModuleFile(llvm::StringRef ModuleName, if (Clang->getDiagnostics().hasErrorOccurred()) return llvm::createStringError("Compilation failed"); - BuiltModuleFiles.addModuleFile(ModuleName, Inputs.CompileCommand.Output); - return llvm::Error::success(); + return ModuleFile{ModuleName, Inputs.CompileCommand.Output}; } + +bool ReusablePrerequisiteModules::canReuse( +const CompilerInvocation &CI, +llvm::IntrusiveRefCntPtr VFS) const { + if (RequiredModules.empty()) +return true; + + SmallVector BMIPaths; + for (auto &MF : RequiredModules) +BMIPaths.push_back(MF->getModuleFilePath()); + return IsModuleFilesUpToDate(BMIPaths, *this, VFS); +} + +class ModuleFileCache { +public: + ModuleFileCache(const GlobalCompilationDatabase &CDB) : CDB(CDB) {} + const GlobalCompilationDatabase &getCDB() const { return CDB; } + + std::shared_ptr getModule(StringRef ModuleName); + + void add(StringRef ModuleName, std::shared_ptr ModuleFile) { +std::lock_guard Lock(ModuleFilesMutex); + +ModuleFiles.insert_or_assign(ModuleName, ModuleFile); + } + + void remove(StringRef ModuleName); + +private: + const GlobalCompilationDatabase &CDB; + + llvm::StringMap> ModuleFiles; + // Mutex to guard accesses to ModuleFiles. + std::mutex ModuleFilesMutex; +}; + +std::shared_ptr +ModuleFileCache::getModule(StringRef ModuleName) { + std::lock_guard Lock(ModuleFilesMutex); + + auto Iter = ModuleFiles.find(ModuleName); + if (Iter == ModuleFiles.end()) +return nullptr; + + if (Iter->second.expired()) { +ModuleFiles.erase(Iter); +return nullptr; + } + + return Iter->second.lock(); +} + +void ModuleFileCache::remove(StringRef ModuleName) { + std::lock_guard Lock(ModuleFilesMutex); + + auto Iter = ModuleFiles.find(ModuleName); + if (Iter == ModuleFiles.end()) +return; + + ModuleFiles.erase(Iter); +} + +/// Collect the directly and indirectly required module names for \param +/// ModuleName in topological order. The \param ModuleName is guaranteed to +/// be the last element in \param ModuleNames. +llvm::SmallVector getAllRequiredModules(ProjectModules &MDB, + StringRef ModuleName) { + llvm::SmallVector ModuleNames; + llvm::StringSet<> ModuleNamesSet; + + auto traversal = [&](StringRef ModuleName, auto recursionHelper) -> void { ChuanqiXu9 wrote: Done https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
@@ -316,36 +295,187 @@ llvm::Error buildModuleFile(llvm::StringRef ModuleName, if (Clang->getDiagnostics().hasErrorOccurred()) return llvm::createStringError("Compilation failed"); - BuiltModuleFiles.addModuleFile(ModuleName, Inputs.CompileCommand.Output); - return llvm::Error::success(); + return ModuleFile{ModuleName, Inputs.CompileCommand.Output}; } + +bool ReusablePrerequisiteModules::canReuse( +const CompilerInvocation &CI, +llvm::IntrusiveRefCntPtr VFS) const { + if (RequiredModules.empty()) +return true; + + SmallVector BMIPaths; + for (auto &MF : RequiredModules) +BMIPaths.push_back(MF->getModuleFilePath()); + return IsModuleFilesUpToDate(BMIPaths, *this, VFS); +} + +class ModuleFileCache { +public: + ModuleFileCache(const GlobalCompilationDatabase &CDB) : CDB(CDB) {} + const GlobalCompilationDatabase &getCDB() const { return CDB; } + + std::shared_ptr getModule(StringRef ModuleName); + + void add(StringRef ModuleName, std::shared_ptr ModuleFile) { +std::lock_guard Lock(ModuleFilesMutex); + +ModuleFiles.insert_or_assign(ModuleName, ModuleFile); ChuanqiXu9 wrote: Done https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
https://github.com/ChuanqiXu9 commented: Thanks for reviewing! https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
@@ -316,36 +295,187 @@ llvm::Error buildModuleFile(llvm::StringRef ModuleName, if (Clang->getDiagnostics().hasErrorOccurred()) return llvm::createStringError("Compilation failed"); - BuiltModuleFiles.addModuleFile(ModuleName, Inputs.CompileCommand.Output); - return llvm::Error::success(); + return ModuleFile{ModuleName, Inputs.CompileCommand.Output}; } + +bool ReusablePrerequisiteModules::canReuse( +const CompilerInvocation &CI, +llvm::IntrusiveRefCntPtr VFS) const { + if (RequiredModules.empty()) +return true; + + SmallVector BMIPaths; ChuanqiXu9 wrote: Done https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
@@ -316,36 +295,187 @@ llvm::Error buildModuleFile(llvm::StringRef ModuleName, if (Clang->getDiagnostics().hasErrorOccurred()) return llvm::createStringError("Compilation failed"); - BuiltModuleFiles.addModuleFile(ModuleName, Inputs.CompileCommand.Output); - return llvm::Error::success(); + return ModuleFile{ModuleName, Inputs.CompileCommand.Output}; } + +bool ReusablePrerequisiteModules::canReuse( +const CompilerInvocation &CI, +llvm::IntrusiveRefCntPtr VFS) const { + if (RequiredModules.empty()) +return true; + + SmallVector BMIPaths; + for (auto &MF : RequiredModules) +BMIPaths.push_back(MF->getModuleFilePath()); + return IsModuleFilesUpToDate(BMIPaths, *this, VFS); +} + +class ModuleFileCache { +public: + ModuleFileCache(const GlobalCompilationDatabase &CDB) : CDB(CDB) {} + const GlobalCompilationDatabase &getCDB() const { return CDB; } + + std::shared_ptr getModule(StringRef ModuleName); + + void add(StringRef ModuleName, std::shared_ptr ModuleFile) { +std::lock_guard Lock(ModuleFilesMutex); + +ModuleFiles.insert_or_assign(ModuleName, ModuleFile); + } + + void remove(StringRef ModuleName); + +private: + const GlobalCompilationDatabase &CDB; + + llvm::StringMap> ModuleFiles; + // Mutex to guard accesses to ModuleFiles. + std::mutex ModuleFilesMutex; +}; + +std::shared_ptr +ModuleFileCache::getModule(StringRef ModuleName) { + std::lock_guard Lock(ModuleFilesMutex); + + auto Iter = ModuleFiles.find(ModuleName); + if (Iter == ModuleFiles.end()) +return nullptr; + + if (Iter->second.expired()) { +ModuleFiles.erase(Iter); +return nullptr; + } + + return Iter->second.lock(); ChuanqiXu9 wrote: Done https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
https://github.com/ChuanqiXu9 updated https://github.com/llvm/llvm-project/pull/106683 >From 15aa6af1f5d22e4b837e8e2fd49469310ffbe7f1 Mon Sep 17 00:00:00 2001 From: Chuanqi Xu Date: Fri, 30 Aug 2024 15:11:07 +0800 Subject: [PATCH 1/6] [clangd] [Modules] Support Reusable Modules Builder --- clang-tools-extra/clangd/ModulesBuilder.cpp | 375 ++ clang-tools-extra/clangd/ModulesBuilder.h | 8 +- .../unittests/PrerequisiteModulesTest.cpp | 36 ++ 3 files changed, 344 insertions(+), 75 deletions(-) diff --git a/clang-tools-extra/clangd/ModulesBuilder.cpp b/clang-tools-extra/clangd/ModulesBuilder.cpp index 97f67ddf5495a2..c4fb3d4010d370 100644 --- a/clang-tools-extra/clangd/ModulesBuilder.cpp +++ b/clang-tools-extra/clangd/ModulesBuilder.cpp @@ -13,6 +13,7 @@ #include "clang/Frontend/FrontendActions.h" #include "clang/Serialization/ASTReader.h" #include "clang/Serialization/InMemoryModuleCache.h" +#include "llvm/ADT/ScopeExit.h" namespace clang { namespace clangd { @@ -192,33 +193,27 @@ bool IsModuleFilesUpToDate( }); } -// StandalonePrerequisiteModules - stands for PrerequisiteModules for which all +// ReusablePrerequisiteModules - stands for PrerequisiteModules for which all // the required modules are built successfully. All the module files -// are owned by the StandalonePrerequisiteModules class. -// -// Any of the built module files won't be shared with other instances of the -// class. So that we can avoid worrying thread safety. -// -// We don't need to worry about duplicated module names here since the standard -// guarantees the module names should be unique to a program. -class StandalonePrerequisiteModules : public PrerequisiteModules { +// are owned by the modules builder. +class ReusablePrerequisiteModules : public PrerequisiteModules { public: - StandalonePrerequisiteModules() = default; + ReusablePrerequisiteModules() = default; - StandalonePrerequisiteModules(const StandalonePrerequisiteModules &) = delete; - StandalonePrerequisiteModules - operator=(const StandalonePrerequisiteModules &) = delete; - StandalonePrerequisiteModules(StandalonePrerequisiteModules &&) = delete; - StandalonePrerequisiteModules - operator=(StandalonePrerequisiteModules &&) = delete; + ReusablePrerequisiteModules(const ReusablePrerequisiteModules &) = delete; + ReusablePrerequisiteModules + operator=(const ReusablePrerequisiteModules &) = delete; + ReusablePrerequisiteModules(ReusablePrerequisiteModules &&) = delete; + ReusablePrerequisiteModules + operator=(ReusablePrerequisiteModules &&) = delete; - ~StandalonePrerequisiteModules() override = default; + ~ReusablePrerequisiteModules() override = default; void adjustHeaderSearchOptions(HeaderSearchOptions &Options) const override { // Appending all built module files. for (auto &RequiredModule : RequiredModules) Options.PrebuiltModuleFiles.insert_or_assign( - RequiredModule.ModuleName, RequiredModule.ModuleFilePath); + RequiredModule->ModuleName, RequiredModule->ModuleFilePath); } bool canReuse(const CompilerInvocation &CI, @@ -228,54 +223,30 @@ class StandalonePrerequisiteModules : public PrerequisiteModules { return BuiltModuleNames.contains(ModuleName); } - void addModuleFile(llvm::StringRef ModuleName, - llvm::StringRef ModuleFilePath) { -RequiredModules.emplace_back(ModuleName, ModuleFilePath); -BuiltModuleNames.insert(ModuleName); + void addModuleFile(std::shared_ptr BMI) { +BuiltModuleNames.insert(BMI->ModuleName); +RequiredModules.emplace_back(std::move(BMI)); } private: - llvm::SmallVector RequiredModules; + mutable llvm::SmallVector, 8> RequiredModules; // A helper class to speedup the query if a module is built. llvm::StringSet<> BuiltModuleNames; }; -// Build a module file for module with `ModuleName`. The information of built -// module file are stored in \param BuiltModuleFiles. -llvm::Error buildModuleFile(llvm::StringRef ModuleName, -const GlobalCompilationDatabase &CDB, -const ThreadsafeFS &TFS, ProjectModules &MDB, -PathRef ModuleFilesPrefix, -StandalonePrerequisiteModules &BuiltModuleFiles) { - if (BuiltModuleFiles.isModuleUnitBuilt(ModuleName)) -return llvm::Error::success(); - - PathRef ModuleUnitFileName = MDB.getSourceForModuleName(ModuleName); - // It is possible that we're meeting third party modules (modules whose - // source are not in the project. e.g, the std module may be a third-party - // module for most projects) or something wrong with the implementation of - // ProjectModules. - // FIXME: How should we treat third party modules here? If we want to ignore - // third party modules, we should return true instead of false here. - // Currently we simply bail out. - if (ModuleUnitFileName.empty()) -return llvm::createStringErro
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
@@ -316,36 +295,187 @@ llvm::Error buildModuleFile(llvm::StringRef ModuleName, if (Clang->getDiagnostics().hasErrorOccurred()) return llvm::createStringError("Compilation failed"); - BuiltModuleFiles.addModuleFile(ModuleName, Inputs.CompileCommand.Output); - return llvm::Error::success(); + return ModuleFile{ModuleName, Inputs.CompileCommand.Output}; } + +bool ReusablePrerequisiteModules::canReuse( +const CompilerInvocation &CI, +llvm::IntrusiveRefCntPtr VFS) const { + if (RequiredModules.empty()) +return true; + + SmallVector BMIPaths; + for (auto &MF : RequiredModules) +BMIPaths.push_back(MF->getModuleFilePath()); + return IsModuleFilesUpToDate(BMIPaths, *this, VFS); +} + +class ModuleFileCache { +public: + ModuleFileCache(const GlobalCompilationDatabase &CDB) : CDB(CDB) {} + const GlobalCompilationDatabase &getCDB() const { return CDB; } + + std::shared_ptr getModule(StringRef ModuleName); + + void add(StringRef ModuleName, std::shared_ptr ModuleFile) { +std::lock_guard Lock(ModuleFilesMutex); + +ModuleFiles.insert_or_assign(ModuleName, ModuleFile); + } + + void remove(StringRef ModuleName); + +private: + const GlobalCompilationDatabase &CDB; + + llvm::StringMap> ModuleFiles; + // Mutex to guard accesses to ModuleFiles. + std::mutex ModuleFilesMutex; +}; + +std::shared_ptr +ModuleFileCache::getModule(StringRef ModuleName) { + std::lock_guard Lock(ModuleFilesMutex); + + auto Iter = ModuleFiles.find(ModuleName); + if (Iter == ModuleFiles.end()) +return nullptr; + + if (Iter->second.expired()) { +ModuleFiles.erase(Iter); +return nullptr; + } + + return Iter->second.lock(); +} + +void ModuleFileCache::remove(StringRef ModuleName) { + std::lock_guard Lock(ModuleFilesMutex); + + auto Iter = ModuleFiles.find(ModuleName); + if (Iter == ModuleFiles.end()) +return; + + ModuleFiles.erase(Iter); kadircet wrote: just `Modulefiles.erase(ModuleName)` ? https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
@@ -316,36 +295,187 @@ llvm::Error buildModuleFile(llvm::StringRef ModuleName, if (Clang->getDiagnostics().hasErrorOccurred()) return llvm::createStringError("Compilation failed"); - BuiltModuleFiles.addModuleFile(ModuleName, Inputs.CompileCommand.Output); - return llvm::Error::success(); + return ModuleFile{ModuleName, Inputs.CompileCommand.Output}; } + +bool ReusablePrerequisiteModules::canReuse( +const CompilerInvocation &CI, +llvm::IntrusiveRefCntPtr VFS) const { + if (RequiredModules.empty()) +return true; + + SmallVector BMIPaths; + for (auto &MF : RequiredModules) +BMIPaths.push_back(MF->getModuleFilePath()); + return IsModuleFilesUpToDate(BMIPaths, *this, VFS); +} + +class ModuleFileCache { +public: + ModuleFileCache(const GlobalCompilationDatabase &CDB) : CDB(CDB) {} + const GlobalCompilationDatabase &getCDB() const { return CDB; } + + std::shared_ptr getModule(StringRef ModuleName); + + void add(StringRef ModuleName, std::shared_ptr ModuleFile) { +std::lock_guard Lock(ModuleFilesMutex); + +ModuleFiles.insert_or_assign(ModuleName, ModuleFile); + } + + void remove(StringRef ModuleName); + +private: + const GlobalCompilationDatabase &CDB; + + llvm::StringMap> ModuleFiles; + // Mutex to guard accesses to ModuleFiles. + std::mutex ModuleFilesMutex; +}; + +std::shared_ptr +ModuleFileCache::getModule(StringRef ModuleName) { + std::lock_guard Lock(ModuleFilesMutex); + + auto Iter = ModuleFiles.find(ModuleName); + if (Iter == ModuleFiles.end()) +return nullptr; + + if (Iter->second.expired()) { +ModuleFiles.erase(Iter); +return nullptr; + } + + return Iter->second.lock(); +} + +void ModuleFileCache::remove(StringRef ModuleName) { + std::lock_guard Lock(ModuleFilesMutex); + + auto Iter = ModuleFiles.find(ModuleName); + if (Iter == ModuleFiles.end()) +return; + + ModuleFiles.erase(Iter); +} + +/// Collect the directly and indirectly required module names for \param +/// ModuleName in topological order. The \param ModuleName is guaranteed to +/// be the last element in \param ModuleNames. +llvm::SmallVector getAllRequiredModules(ProjectModules &MDB, + StringRef ModuleName) { + llvm::SmallVector ModuleNames; + llvm::StringSet<> ModuleNamesSet; + + auto traversal = [&](StringRef ModuleName, auto recursionHelper) -> void { +ModuleNamesSet.insert(ModuleName); + +for (StringRef RequiredModuleName : + MDB.getRequiredModules(MDB.getSourceForModuleName(ModuleName))) + if (ModuleNamesSet.insert(RequiredModuleName).second) +recursionHelper(RequiredModuleName, recursionHelper); + +ModuleNames.push_back(ModuleName); + }; + traversal(ModuleName, traversal); + + return ModuleNames; +} + } // namespace +class ModulesBuilder::ModulesBuilderImpl { +public: + ModulesBuilderImpl(const GlobalCompilationDatabase &CDB) : Cache(CDB) {} + + const GlobalCompilationDatabase &getCDB() const { return Cache.getCDB(); } + + llvm::Error + getOrBuildModuleFile(StringRef ModuleName, const ThreadsafeFS &TFS, + ProjectModules &MDB, + ReusablePrerequisiteModules &BuiltModuleFiles); + +private: + ModuleFileCache Cache; +}; + +llvm::Error ModulesBuilder::ModulesBuilderImpl::getOrBuildModuleFile( +StringRef ModuleName, const ThreadsafeFS &TFS, ProjectModules &MDB, +ReusablePrerequisiteModules &BuiltModuleFiles) { + if (BuiltModuleFiles.isModuleUnitBuilt(ModuleName)) +return llvm::Error::success(); + + PathRef ModuleUnitFileName = MDB.getSourceForModuleName(ModuleName); + /// It is possible that we're meeting third party modules (modules whose + /// source are not in the project. e.g, the std module may be a third-party + /// module for most project) or something wrong with the implementation of + /// ProjectModules. + /// FIXME: How should we treat third party modules here? If we want to ignore + /// third party modules, we should return true instead of false here. + /// Currently we simply bail out. + if (ModuleUnitFileName.empty()) +return llvm::createStringError( +llvm::formatv("Don't get the module unit for module {0}", ModuleName)); + + // Get Required modules in topological order. + auto ReqModuleNames = getAllRequiredModules(MDB, ModuleName); + for (llvm::StringRef ReqModuleName : ReqModuleNames) { +if (BuiltModuleFiles.isModuleUnitBuilt(ModuleName)) + continue; + +if (auto Cached = Cache.getModule(ReqModuleName)) { + if (IsModuleFileUpToDate(Cached->getModuleFilePath(), BuiltModuleFiles, + TFS.view(std::nullopt))) { +log("Reusing module {0} from {1}", ModuleName, +Cached->getModuleFilePath()); +BuiltModuleFiles.addModuleFile(std::move(Cached)); +continue; + } + Cache.remove(ReqModuleName); +} + +llvm::SmallString<256> ModuleFilesPrefix = +getUniqueModuleFilesPa
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
@@ -316,36 +295,187 @@ llvm::Error buildModuleFile(llvm::StringRef ModuleName, if (Clang->getDiagnostics().hasErrorOccurred()) return llvm::createStringError("Compilation failed"); - BuiltModuleFiles.addModuleFile(ModuleName, Inputs.CompileCommand.Output); - return llvm::Error::success(); + return ModuleFile{ModuleName, Inputs.CompileCommand.Output}; } + +bool ReusablePrerequisiteModules::canReuse( +const CompilerInvocation &CI, +llvm::IntrusiveRefCntPtr VFS) const { + if (RequiredModules.empty()) +return true; + + SmallVector BMIPaths; + for (auto &MF : RequiredModules) +BMIPaths.push_back(MF->getModuleFilePath()); + return IsModuleFilesUpToDate(BMIPaths, *this, VFS); +} + +class ModuleFileCache { +public: + ModuleFileCache(const GlobalCompilationDatabase &CDB) : CDB(CDB) {} + const GlobalCompilationDatabase &getCDB() const { return CDB; } + + std::shared_ptr getModule(StringRef ModuleName); + + void add(StringRef ModuleName, std::shared_ptr ModuleFile) { +std::lock_guard Lock(ModuleFilesMutex); + +ModuleFiles.insert_or_assign(ModuleName, ModuleFile); + } + + void remove(StringRef ModuleName); + +private: + const GlobalCompilationDatabase &CDB; + + llvm::StringMap> ModuleFiles; + // Mutex to guard accesses to ModuleFiles. + std::mutex ModuleFilesMutex; +}; + +std::shared_ptr +ModuleFileCache::getModule(StringRef ModuleName) { + std::lock_guard Lock(ModuleFilesMutex); + + auto Iter = ModuleFiles.find(ModuleName); + if (Iter == ModuleFiles.end()) +return nullptr; + + if (Iter->second.expired()) { +ModuleFiles.erase(Iter); +return nullptr; + } + + return Iter->second.lock(); kadircet wrote: again we can actually lose the reference between `expired` and `lock` call here (e.g. another thread destroys the last reference right after we called `expired`). can you instead do: ```cpp if (auto Res = Iter->second.lock()) return Res; ModuleFiles.erase(Iter); return nullptr; ``` here `lock` atomically makes a copy, if weak_ptr hasn't expired. https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
https://github.com/kadircet edited https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
@@ -316,36 +295,187 @@ llvm::Error buildModuleFile(llvm::StringRef ModuleName, if (Clang->getDiagnostics().hasErrorOccurred()) return llvm::createStringError("Compilation failed"); - BuiltModuleFiles.addModuleFile(ModuleName, Inputs.CompileCommand.Output); - return llvm::Error::success(); + return ModuleFile{ModuleName, Inputs.CompileCommand.Output}; } + +bool ReusablePrerequisiteModules::canReuse( +const CompilerInvocation &CI, +llvm::IntrusiveRefCntPtr VFS) const { + if (RequiredModules.empty()) +return true; + + SmallVector BMIPaths; + for (auto &MF : RequiredModules) +BMIPaths.push_back(MF->getModuleFilePath()); + return IsModuleFilesUpToDate(BMIPaths, *this, VFS); +} + +class ModuleFileCache { +public: + ModuleFileCache(const GlobalCompilationDatabase &CDB) : CDB(CDB) {} + const GlobalCompilationDatabase &getCDB() const { return CDB; } + + std::shared_ptr getModule(StringRef ModuleName); + + void add(StringRef ModuleName, std::shared_ptr ModuleFile) { +std::lock_guard Lock(ModuleFilesMutex); + +ModuleFiles.insert_or_assign(ModuleName, ModuleFile); kadircet wrote: nit: `ModuleFiles[ModuleName] = std::move(ModuleFile);` https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
@@ -316,36 +295,187 @@ llvm::Error buildModuleFile(llvm::StringRef ModuleName, if (Clang->getDiagnostics().hasErrorOccurred()) return llvm::createStringError("Compilation failed"); - BuiltModuleFiles.addModuleFile(ModuleName, Inputs.CompileCommand.Output); - return llvm::Error::success(); + return ModuleFile{ModuleName, Inputs.CompileCommand.Output}; } + +bool ReusablePrerequisiteModules::canReuse( +const CompilerInvocation &CI, +llvm::IntrusiveRefCntPtr VFS) const { + if (RequiredModules.empty()) +return true; + + SmallVector BMIPaths; + for (auto &MF : RequiredModules) +BMIPaths.push_back(MF->getModuleFilePath()); + return IsModuleFilesUpToDate(BMIPaths, *this, VFS); +} + +class ModuleFileCache { +public: + ModuleFileCache(const GlobalCompilationDatabase &CDB) : CDB(CDB) {} + const GlobalCompilationDatabase &getCDB() const { return CDB; } + + std::shared_ptr getModule(StringRef ModuleName); + + void add(StringRef ModuleName, std::shared_ptr ModuleFile) { +std::lock_guard Lock(ModuleFilesMutex); + +ModuleFiles.insert_or_assign(ModuleName, ModuleFile); + } + + void remove(StringRef ModuleName); + +private: + const GlobalCompilationDatabase &CDB; + + llvm::StringMap> ModuleFiles; + // Mutex to guard accesses to ModuleFiles. + std::mutex ModuleFilesMutex; +}; + +std::shared_ptr +ModuleFileCache::getModule(StringRef ModuleName) { + std::lock_guard Lock(ModuleFilesMutex); + + auto Iter = ModuleFiles.find(ModuleName); + if (Iter == ModuleFiles.end()) +return nullptr; + + if (Iter->second.expired()) { +ModuleFiles.erase(Iter); +return nullptr; + } + + return Iter->second.lock(); +} + +void ModuleFileCache::remove(StringRef ModuleName) { + std::lock_guard Lock(ModuleFilesMutex); + + auto Iter = ModuleFiles.find(ModuleName); + if (Iter == ModuleFiles.end()) +return; + + ModuleFiles.erase(Iter); +} + +/// Collect the directly and indirectly required module names for \param +/// ModuleName in topological order. The \param ModuleName is guaranteed to +/// be the last element in \param ModuleNames. +llvm::SmallVector getAllRequiredModules(ProjectModules &MDB, + StringRef ModuleName) { + llvm::SmallVector ModuleNames; + llvm::StringSet<> ModuleNamesSet; + + auto traversal = [&](StringRef ModuleName, auto recursionHelper) -> void { kadircet wrote: s/traversal/VisitDeps/ s/recursionHelper/Visitor/ https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
@@ -316,36 +295,187 @@ llvm::Error buildModuleFile(llvm::StringRef ModuleName, if (Clang->getDiagnostics().hasErrorOccurred()) return llvm::createStringError("Compilation failed"); - BuiltModuleFiles.addModuleFile(ModuleName, Inputs.CompileCommand.Output); - return llvm::Error::success(); + return ModuleFile{ModuleName, Inputs.CompileCommand.Output}; } + +bool ReusablePrerequisiteModules::canReuse( +const CompilerInvocation &CI, +llvm::IntrusiveRefCntPtr VFS) const { + if (RequiredModules.empty()) +return true; + + SmallVector BMIPaths; kadircet wrote: nit: `llvm::SmallVector` and `llvm::StringRef` https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
@@ -316,36 +295,187 @@ llvm::Error buildModuleFile(llvm::StringRef ModuleName, if (Clang->getDiagnostics().hasErrorOccurred()) return llvm::createStringError("Compilation failed"); - BuiltModuleFiles.addModuleFile(ModuleName, Inputs.CompileCommand.Output); - return llvm::Error::success(); + return ModuleFile{ModuleName, Inputs.CompileCommand.Output}; } + +bool ReusablePrerequisiteModules::canReuse( +const CompilerInvocation &CI, +llvm::IntrusiveRefCntPtr VFS) const { + if (RequiredModules.empty()) +return true; + + SmallVector BMIPaths; + for (auto &MF : RequiredModules) +BMIPaths.push_back(MF->getModuleFilePath()); + return IsModuleFilesUpToDate(BMIPaths, *this, VFS); +} + +class ModuleFileCache { +public: + ModuleFileCache(const GlobalCompilationDatabase &CDB) : CDB(CDB) {} + const GlobalCompilationDatabase &getCDB() const { return CDB; } + + std::shared_ptr getModule(StringRef ModuleName); + + void add(StringRef ModuleName, std::shared_ptr ModuleFile) { +std::lock_guard Lock(ModuleFilesMutex); + +ModuleFiles.insert_or_assign(ModuleName, ModuleFile); + } + + void remove(StringRef ModuleName); + +private: + const GlobalCompilationDatabase &CDB; + + llvm::StringMap> ModuleFiles; + // Mutex to guard accesses to ModuleFiles. + std::mutex ModuleFilesMutex; +}; + +std::shared_ptr +ModuleFileCache::getModule(StringRef ModuleName) { + std::lock_guard Lock(ModuleFilesMutex); + + auto Iter = ModuleFiles.find(ModuleName); + if (Iter == ModuleFiles.end()) +return nullptr; + + if (Iter->second.expired()) { +ModuleFiles.erase(Iter); +return nullptr; + } + + return Iter->second.lock(); +} + +void ModuleFileCache::remove(StringRef ModuleName) { + std::lock_guard Lock(ModuleFilesMutex); + + auto Iter = ModuleFiles.find(ModuleName); + if (Iter == ModuleFiles.end()) +return; + + ModuleFiles.erase(Iter); +} + +/// Collect the directly and indirectly required module names for \param +/// ModuleName in topological order. The \param ModuleName is guaranteed to +/// be the last element in \param ModuleNames. +llvm::SmallVector getAllRequiredModules(ProjectModules &MDB, + StringRef ModuleName) { + llvm::SmallVector ModuleNames; + llvm::StringSet<> ModuleNamesSet; + + auto traversal = [&](StringRef ModuleName, auto recursionHelper) -> void { +ModuleNamesSet.insert(ModuleName); + +for (StringRef RequiredModuleName : + MDB.getRequiredModules(MDB.getSourceForModuleName(ModuleName))) + if (ModuleNamesSet.insert(RequiredModuleName).second) +recursionHelper(RequiredModuleName, recursionHelper); + +ModuleNames.push_back(ModuleName); + }; + traversal(ModuleName, traversal); + + return ModuleNames; +} + } // namespace +class ModulesBuilder::ModulesBuilderImpl { +public: + ModulesBuilderImpl(const GlobalCompilationDatabase &CDB) : Cache(CDB) {} + + const GlobalCompilationDatabase &getCDB() const { return Cache.getCDB(); } + + llvm::Error + getOrBuildModuleFile(StringRef ModuleName, const ThreadsafeFS &TFS, + ProjectModules &MDB, + ReusablePrerequisiteModules &BuiltModuleFiles); + +private: + ModuleFileCache Cache; +}; + +llvm::Error ModulesBuilder::ModulesBuilderImpl::getOrBuildModuleFile( +StringRef ModuleName, const ThreadsafeFS &TFS, ProjectModules &MDB, +ReusablePrerequisiteModules &BuiltModuleFiles) { + if (BuiltModuleFiles.isModuleUnitBuilt(ModuleName)) +return llvm::Error::success(); + + PathRef ModuleUnitFileName = MDB.getSourceForModuleName(ModuleName); + /// It is possible that we're meeting third party modules (modules whose + /// source are not in the project. e.g, the std module may be a third-party + /// module for most project) or something wrong with the implementation of + /// ProjectModules. + /// FIXME: How should we treat third party modules here? If we want to ignore + /// third party modules, we should return true instead of false here. + /// Currently we simply bail out. + if (ModuleUnitFileName.empty()) +return llvm::createStringError( +llvm::formatv("Don't get the module unit for module {0}", ModuleName)); + + // Get Required modules in topological order. + auto ReqModuleNames = getAllRequiredModules(MDB, ModuleName); + for (llvm::StringRef ReqModuleName : ReqModuleNames) { +if (BuiltModuleFiles.isModuleUnitBuilt(ModuleName)) + continue; + +if (auto Cached = Cache.getModule(ReqModuleName)) { + if (IsModuleFileUpToDate(Cached->getModuleFilePath(), BuiltModuleFiles, + TFS.view(std::nullopt))) { +log("Reusing module {0} from {1}", ModuleName, +Cached->getModuleFilePath()); +BuiltModuleFiles.addModuleFile(std::move(Cached)); +continue; + } + Cache.remove(ReqModuleName); +} + +llvm::SmallString<256> ModuleFilesPrefix = +getUniqueModuleFilesPa
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
https://github.com/kadircet requested changes to this pull request. thanks, i think this looks great! some little comments/nits. https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
@@ -316,36 +309,221 @@ llvm::Error buildModuleFile(llvm::StringRef ModuleName, if (Clang->getDiagnostics().hasErrorOccurred()) return llvm::createStringError("Compilation failed"); - BuiltModuleFiles.addModuleFile(ModuleName, Inputs.CompileCommand.Output); - return llvm::Error::success(); + return ModuleFile{ModuleName, Inputs.CompileCommand.Output, BuiltModuleFiles}; +} + +bool ReusablePrerequisiteModules::canReuse( +const CompilerInvocation &CI, +llvm::IntrusiveRefCntPtr VFS) const { + if (RequiredModules.empty()) +return true; + + SmallVector BMIPaths; + for (auto &MF : RequiredModules) +BMIPaths.push_back(MF->getModuleFilePath()); + return IsModuleFilesUpToDate(BMIPaths, *this, VFS); +} + +class ModuleFileCache { +public: + ModuleFileCache(const GlobalCompilationDatabase &CDB) : CDB(CDB) {} + + llvm::Error + getOrBuildModuleFile(StringRef ModuleName, const ThreadsafeFS &TFS, + ProjectModules &MDB, + ReusablePrerequisiteModules &RequiredModules); + const GlobalCompilationDatabase &getCDB() const { return CDB; } + + std::shared_ptr + getValidModuleFile(StringRef ModuleName, ProjectModules &MDB, + const ThreadsafeFS &TFS, + PrerequisiteModules &BuiltModuleFiles); + + void add(StringRef ModuleName, std::shared_ptr ModuleFile) { +std::lock_guard Lock(ModuleFilesMutex); + +ModuleFiles.insert_or_assign(ModuleName, ModuleFile); + } + +private: + const GlobalCompilationDatabase &CDB; + + llvm::StringMap> ModuleFiles; + // Mutex to guard accesses to ModuleFiles. + std::mutex ModuleFilesMutex; +}; + +/// Collect the directly and indirectly required module names for \param +/// ModuleName. The \param ModuleName is guaranteed to be the first element in +/// \param ModuleNames. +llvm::SmallVector getAllRequiredModules(ProjectModules &MDB, + StringRef ModuleName) { + llvm::SmallVector ModuleNames; + + std::queue Worklist; + llvm::StringSet<> ModuleNamesSet; + Worklist.push(ModuleName); + + while (!Worklist.empty()) { +StringRef CurrentModule = Worklist.front(); +Worklist.pop(); + +if (!ModuleNamesSet.insert(CurrentModule).second) + continue; + +ModuleNames.push_back(CurrentModule); + +for (StringRef RequiredModuleName : + MDB.getRequiredModules(MDB.getSourceForModuleName(CurrentModule))) + if (!ModuleNamesSet.contains(RequiredModuleName)) +Worklist.push(RequiredModuleName); + } + + return ModuleNames; +} + +std::shared_ptr +ModuleFileCache::getValidModuleFile(StringRef ModuleName, ProjectModules &MDB, +const ThreadsafeFS &TFS, +PrerequisiteModules &BuiltModuleFiles) { + { +std::lock_guard Lock(ModuleFilesMutex); + +auto Iter = ModuleFiles.find(ModuleName); +if (Iter == ModuleFiles.end()) + return nullptr; + +if (Iter->second.expired()) { + ModuleFiles.erase(Iter); + return nullptr; +} + } + + llvm::SmallVector ModuleNames = + getAllRequiredModules(MDB, ModuleName); + + llvm::SmallVector> RequiredModuleFiles; + + { +std::lock_guard Lock(ModuleFilesMutex); + +for (StringRef ModuleName : ModuleNames) { + auto Iter = ModuleFiles.find(ModuleName); + if (Iter == ModuleFiles.end()) +return nullptr; + + if (Iter->second.expired()) { +ModuleFiles.erase(Iter); +return nullptr; + } + + RequiredModuleFiles.push_back(Iter->second.lock()); +} + } + + assert(!RequiredModuleFiles.empty()); + + if (llvm::any_of(RequiredModuleFiles, + [&](std::shared_ptr MF) { + return !IsModuleFileUpToDate(MF->getModuleFilePath(), + BuiltModuleFiles, + TFS.view(std::nullopt)); + })) +return nullptr; + + return RequiredModuleFiles[0]; } } // namespace +class ModulesBuilder::ModulesBuilderImpl { +public: + ModulesBuilderImpl(const GlobalCompilationDatabase &CDB) : Cache(CDB) {} + + const GlobalCompilationDatabase &getCDB() const { return Cache.getCDB(); } + + llvm::Error + getOrBuildModuleFile(StringRef ModuleName, const ThreadsafeFS &TFS, + ProjectModules &MDB, + ReusablePrerequisiteModules &BuiltModuleFiles); + +private: + ModuleFileCache Cache; +}; + +llvm::Error ModulesBuilder::ModulesBuilderImpl::getOrBuildModuleFile( +StringRef ModuleName, const ThreadsafeFS &TFS, ProjectModules &MDB, +ReusablePrerequisiteModules &BuiltModuleFiles) { + if (BuiltModuleFiles.isModuleUnitBuilt(ModuleName)) +return llvm::Error::success(); + + PathRef ModuleUnitFileName = MDB.getSourceForModuleName(ModuleName); + /// It is possible that we're meeting third party modules (modules whose + /// source are not in the
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
https://github.com/ChuanqiXu9 updated https://github.com/llvm/llvm-project/pull/106683 >From 15aa6af1f5d22e4b837e8e2fd49469310ffbe7f1 Mon Sep 17 00:00:00 2001 From: Chuanqi Xu Date: Fri, 30 Aug 2024 15:11:07 +0800 Subject: [PATCH 1/5] [clangd] [Modules] Support Reusable Modules Builder --- clang-tools-extra/clangd/ModulesBuilder.cpp | 375 ++ clang-tools-extra/clangd/ModulesBuilder.h | 8 +- .../unittests/PrerequisiteModulesTest.cpp | 36 ++ 3 files changed, 344 insertions(+), 75 deletions(-) diff --git a/clang-tools-extra/clangd/ModulesBuilder.cpp b/clang-tools-extra/clangd/ModulesBuilder.cpp index 97f67ddf5495a2..c4fb3d4010d370 100644 --- a/clang-tools-extra/clangd/ModulesBuilder.cpp +++ b/clang-tools-extra/clangd/ModulesBuilder.cpp @@ -13,6 +13,7 @@ #include "clang/Frontend/FrontendActions.h" #include "clang/Serialization/ASTReader.h" #include "clang/Serialization/InMemoryModuleCache.h" +#include "llvm/ADT/ScopeExit.h" namespace clang { namespace clangd { @@ -192,33 +193,27 @@ bool IsModuleFilesUpToDate( }); } -// StandalonePrerequisiteModules - stands for PrerequisiteModules for which all +// ReusablePrerequisiteModules - stands for PrerequisiteModules for which all // the required modules are built successfully. All the module files -// are owned by the StandalonePrerequisiteModules class. -// -// Any of the built module files won't be shared with other instances of the -// class. So that we can avoid worrying thread safety. -// -// We don't need to worry about duplicated module names here since the standard -// guarantees the module names should be unique to a program. -class StandalonePrerequisiteModules : public PrerequisiteModules { +// are owned by the modules builder. +class ReusablePrerequisiteModules : public PrerequisiteModules { public: - StandalonePrerequisiteModules() = default; + ReusablePrerequisiteModules() = default; - StandalonePrerequisiteModules(const StandalonePrerequisiteModules &) = delete; - StandalonePrerequisiteModules - operator=(const StandalonePrerequisiteModules &) = delete; - StandalonePrerequisiteModules(StandalonePrerequisiteModules &&) = delete; - StandalonePrerequisiteModules - operator=(StandalonePrerequisiteModules &&) = delete; + ReusablePrerequisiteModules(const ReusablePrerequisiteModules &) = delete; + ReusablePrerequisiteModules + operator=(const ReusablePrerequisiteModules &) = delete; + ReusablePrerequisiteModules(ReusablePrerequisiteModules &&) = delete; + ReusablePrerequisiteModules + operator=(ReusablePrerequisiteModules &&) = delete; - ~StandalonePrerequisiteModules() override = default; + ~ReusablePrerequisiteModules() override = default; void adjustHeaderSearchOptions(HeaderSearchOptions &Options) const override { // Appending all built module files. for (auto &RequiredModule : RequiredModules) Options.PrebuiltModuleFiles.insert_or_assign( - RequiredModule.ModuleName, RequiredModule.ModuleFilePath); + RequiredModule->ModuleName, RequiredModule->ModuleFilePath); } bool canReuse(const CompilerInvocation &CI, @@ -228,54 +223,30 @@ class StandalonePrerequisiteModules : public PrerequisiteModules { return BuiltModuleNames.contains(ModuleName); } - void addModuleFile(llvm::StringRef ModuleName, - llvm::StringRef ModuleFilePath) { -RequiredModules.emplace_back(ModuleName, ModuleFilePath); -BuiltModuleNames.insert(ModuleName); + void addModuleFile(std::shared_ptr BMI) { +BuiltModuleNames.insert(BMI->ModuleName); +RequiredModules.emplace_back(std::move(BMI)); } private: - llvm::SmallVector RequiredModules; + mutable llvm::SmallVector, 8> RequiredModules; // A helper class to speedup the query if a module is built. llvm::StringSet<> BuiltModuleNames; }; -// Build a module file for module with `ModuleName`. The information of built -// module file are stored in \param BuiltModuleFiles. -llvm::Error buildModuleFile(llvm::StringRef ModuleName, -const GlobalCompilationDatabase &CDB, -const ThreadsafeFS &TFS, ProjectModules &MDB, -PathRef ModuleFilesPrefix, -StandalonePrerequisiteModules &BuiltModuleFiles) { - if (BuiltModuleFiles.isModuleUnitBuilt(ModuleName)) -return llvm::Error::success(); - - PathRef ModuleUnitFileName = MDB.getSourceForModuleName(ModuleName); - // It is possible that we're meeting third party modules (modules whose - // source are not in the project. e.g, the std module may be a third-party - // module for most projects) or something wrong with the implementation of - // ProjectModules. - // FIXME: How should we treat third party modules here? If we want to ignore - // third party modules, we should return true instead of false here. - // Currently we simply bail out. - if (ModuleUnitFileName.empty()) -return llvm::createStringErro
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
https://github.com/ChuanqiXu9 updated https://github.com/llvm/llvm-project/pull/106683 >From 15aa6af1f5d22e4b837e8e2fd49469310ffbe7f1 Mon Sep 17 00:00:00 2001 From: Chuanqi Xu Date: Fri, 30 Aug 2024 15:11:07 +0800 Subject: [PATCH 1/5] [clangd] [Modules] Support Reusable Modules Builder --- clang-tools-extra/clangd/ModulesBuilder.cpp | 375 ++ clang-tools-extra/clangd/ModulesBuilder.h | 8 +- .../unittests/PrerequisiteModulesTest.cpp | 36 ++ 3 files changed, 344 insertions(+), 75 deletions(-) diff --git a/clang-tools-extra/clangd/ModulesBuilder.cpp b/clang-tools-extra/clangd/ModulesBuilder.cpp index 97f67ddf5495a2..c4fb3d4010d370 100644 --- a/clang-tools-extra/clangd/ModulesBuilder.cpp +++ b/clang-tools-extra/clangd/ModulesBuilder.cpp @@ -13,6 +13,7 @@ #include "clang/Frontend/FrontendActions.h" #include "clang/Serialization/ASTReader.h" #include "clang/Serialization/InMemoryModuleCache.h" +#include "llvm/ADT/ScopeExit.h" namespace clang { namespace clangd { @@ -192,33 +193,27 @@ bool IsModuleFilesUpToDate( }); } -// StandalonePrerequisiteModules - stands for PrerequisiteModules for which all +// ReusablePrerequisiteModules - stands for PrerequisiteModules for which all // the required modules are built successfully. All the module files -// are owned by the StandalonePrerequisiteModules class. -// -// Any of the built module files won't be shared with other instances of the -// class. So that we can avoid worrying thread safety. -// -// We don't need to worry about duplicated module names here since the standard -// guarantees the module names should be unique to a program. -class StandalonePrerequisiteModules : public PrerequisiteModules { +// are owned by the modules builder. +class ReusablePrerequisiteModules : public PrerequisiteModules { public: - StandalonePrerequisiteModules() = default; + ReusablePrerequisiteModules() = default; - StandalonePrerequisiteModules(const StandalonePrerequisiteModules &) = delete; - StandalonePrerequisiteModules - operator=(const StandalonePrerequisiteModules &) = delete; - StandalonePrerequisiteModules(StandalonePrerequisiteModules &&) = delete; - StandalonePrerequisiteModules - operator=(StandalonePrerequisiteModules &&) = delete; + ReusablePrerequisiteModules(const ReusablePrerequisiteModules &) = delete; + ReusablePrerequisiteModules + operator=(const ReusablePrerequisiteModules &) = delete; + ReusablePrerequisiteModules(ReusablePrerequisiteModules &&) = delete; + ReusablePrerequisiteModules + operator=(ReusablePrerequisiteModules &&) = delete; - ~StandalonePrerequisiteModules() override = default; + ~ReusablePrerequisiteModules() override = default; void adjustHeaderSearchOptions(HeaderSearchOptions &Options) const override { // Appending all built module files. for (auto &RequiredModule : RequiredModules) Options.PrebuiltModuleFiles.insert_or_assign( - RequiredModule.ModuleName, RequiredModule.ModuleFilePath); + RequiredModule->ModuleName, RequiredModule->ModuleFilePath); } bool canReuse(const CompilerInvocation &CI, @@ -228,54 +223,30 @@ class StandalonePrerequisiteModules : public PrerequisiteModules { return BuiltModuleNames.contains(ModuleName); } - void addModuleFile(llvm::StringRef ModuleName, - llvm::StringRef ModuleFilePath) { -RequiredModules.emplace_back(ModuleName, ModuleFilePath); -BuiltModuleNames.insert(ModuleName); + void addModuleFile(std::shared_ptr BMI) { +BuiltModuleNames.insert(BMI->ModuleName); +RequiredModules.emplace_back(std::move(BMI)); } private: - llvm::SmallVector RequiredModules; + mutable llvm::SmallVector, 8> RequiredModules; // A helper class to speedup the query if a module is built. llvm::StringSet<> BuiltModuleNames; }; -// Build a module file for module with `ModuleName`. The information of built -// module file are stored in \param BuiltModuleFiles. -llvm::Error buildModuleFile(llvm::StringRef ModuleName, -const GlobalCompilationDatabase &CDB, -const ThreadsafeFS &TFS, ProjectModules &MDB, -PathRef ModuleFilesPrefix, -StandalonePrerequisiteModules &BuiltModuleFiles) { - if (BuiltModuleFiles.isModuleUnitBuilt(ModuleName)) -return llvm::Error::success(); - - PathRef ModuleUnitFileName = MDB.getSourceForModuleName(ModuleName); - // It is possible that we're meeting third party modules (modules whose - // source are not in the project. e.g, the std module may be a third-party - // module for most projects) or something wrong with the implementation of - // ProjectModules. - // FIXME: How should we treat third party modules here? If we want to ignore - // third party modules, we should return true instead of false here. - // Currently we simply bail out. - if (ModuleUnitFileName.empty()) -return llvm::createStringErro
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
@@ -316,36 +294,205 @@ llvm::Error buildModuleFile(llvm::StringRef ModuleName, if (Clang->getDiagnostics().hasErrorOccurred()) return llvm::createStringError("Compilation failed"); - BuiltModuleFiles.addModuleFile(ModuleName, Inputs.CompileCommand.Output); - return llvm::Error::success(); + return ModuleFile{ModuleName, Inputs.CompileCommand.Output}; +} + +bool ReusablePrerequisiteModules::canReuse( +const CompilerInvocation &CI, +llvm::IntrusiveRefCntPtr VFS) const { + if (RequiredModules.empty()) +return true; + + SmallVector BMIPaths; + for (auto &MF : RequiredModules) +BMIPaths.push_back(MF->getModuleFilePath()); + return IsModuleFilesUpToDate(BMIPaths, *this, VFS); +} + +class ModuleFileCache { +public: + ModuleFileCache(const GlobalCompilationDatabase &CDB) : CDB(CDB) {} + + llvm::Error + getOrBuildModuleFile(StringRef ModuleName, const ThreadsafeFS &TFS, + ProjectModules &MDB, + ReusablePrerequisiteModules &RequiredModules); + const GlobalCompilationDatabase &getCDB() const { return CDB; } + + std::shared_ptr + getValidModuleFile(StringRef ModuleName, ProjectModules &MDB, + const ThreadsafeFS &TFS, + PrerequisiteModules &BuiltModuleFiles); + + void add(StringRef ModuleName, std::shared_ptr ModuleFile) { +std::lock_guard _(ModuleFilesMutex); + +ModuleFiles.insert_or_assign(ModuleName, ModuleFile); + } + +private: + const GlobalCompilationDatabase &CDB; + + llvm::StringMap> ModuleFiles; + // Mutex to guard accesses to ModuleFiles. + std::mutex ModuleFilesMutex; +}; + +/// Collect the directly and indirectly required module names for \param +/// ModuleName. The \param ModuleName is guaranteed to be the first element in +/// \param ModuleNames. +void getAllRequiredModules(ProjectModules &MDB, StringRef ModuleName, + llvm::SmallVector &ModuleNames) { + std::queue Worklist; + llvm::StringSet<> ModuleNamesSet; + Worklist.push(ModuleName); + + while (!Worklist.empty()) { +StringRef CurrentModule = Worklist.front(); +Worklist.pop(); + +if (!ModuleNamesSet.insert(CurrentModule).second) + continue; + +ModuleNames.push_back(CurrentModule); + +for (StringRef RequiredModuleName : + MDB.getRequiredModules(MDB.getSourceForModuleName(CurrentModule))) + if (!ModuleNamesSet.contains(RequiredModuleName)) +Worklist.push(RequiredModuleName); + } +} + +std::shared_ptr +ModuleFileCache::getValidModuleFile(StringRef ModuleName, ProjectModules &MDB, +const ThreadsafeFS &TFS, +PrerequisiteModules &BuiltModuleFiles) { + { +std::lock_guard _(ModuleFilesMutex); + +if (ModuleFiles.find(ModuleName) == ModuleFiles.end()) + return nullptr; + } + + llvm::SmallVector ModuleNames; + getAllRequiredModules(MDB, ModuleName, ModuleNames); + + llvm::SmallVector> RequiredModuleFiles; + + { +std::lock_guard _(ModuleFilesMutex); + +for (StringRef ModuleName : ModuleNames) { + auto Iter = ModuleFiles.find(ModuleName); + if (Iter == ModuleFiles.end()) +return nullptr; + + RequiredModuleFiles.push_back(Iter->second); +} + } + + if (RequiredModuleFiles.empty()) +return nullptr; ChuanqiXu9 wrote: Done https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
@@ -316,36 +287,169 @@ llvm::Error buildModuleFile(llvm::StringRef ModuleName, if (Clang->getDiagnostics().hasErrorOccurred()) return llvm::createStringError("Compilation failed"); - BuiltModuleFiles.addModuleFile(ModuleName, Inputs.CompileCommand.Output); - return llvm::Error::success(); + return ModuleFile{ModuleName, Inputs.CompileCommand.Output}; +} + +bool ReusablePrerequisiteModules::canReuse( +const CompilerInvocation &CI, +llvm::IntrusiveRefCntPtr VFS) const { + if (RequiredModules.empty()) +return true; + + SmallVector BMIPaths; + for (auto &MF : RequiredModules) +BMIPaths.push_back(MF->ModuleFilePath); + return IsModuleFilesUpToDate(BMIPaths, *this, VFS); } } // namespace +class ModulesBuilder::ModuleFileCache { +public: + ModuleFileCache(const GlobalCompilationDatabase &CDB) : CDB(CDB) {} + + llvm::Error + getOrBuildModuleFile(StringRef ModuleName, const ThreadsafeFS &TFS, + ProjectModules &MDB, + ReusablePrerequisiteModules &RequiredModules); + const GlobalCompilationDatabase &getCDB() const { return CDB; } + +private: + std::shared_ptr + getValidModuleFile(StringRef ModuleName, ProjectModules &MDB, + const ThreadsafeFS &TFS, + PrerequisiteModules &BuiltModuleFiles); + + /// This should only be called by getValidModuleFile. This is unlocked version + /// of getValidModuleFile. The function is extracted to avoid dead locks when + /// recursing. + std::shared_ptr + isValidModuleFileUnlocked(StringRef ModuleName, ProjectModules &MDB, +const ThreadsafeFS &TFS, +PrerequisiteModules &BuiltModuleFiles); + + const GlobalCompilationDatabase &CDB; + + llvm::StringMap> ModuleFiles; ChuanqiXu9 wrote: A just experiment. You're correct here. I've removed the codes. https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
@@ -316,36 +294,205 @@ llvm::Error buildModuleFile(llvm::StringRef ModuleName, if (Clang->getDiagnostics().hasErrorOccurred()) return llvm::createStringError("Compilation failed"); - BuiltModuleFiles.addModuleFile(ModuleName, Inputs.CompileCommand.Output); - return llvm::Error::success(); + return ModuleFile{ModuleName, Inputs.CompileCommand.Output}; +} + +bool ReusablePrerequisiteModules::canReuse( +const CompilerInvocation &CI, +llvm::IntrusiveRefCntPtr VFS) const { + if (RequiredModules.empty()) +return true; + + SmallVector BMIPaths; + for (auto &MF : RequiredModules) +BMIPaths.push_back(MF->getModuleFilePath()); + return IsModuleFilesUpToDate(BMIPaths, *this, VFS); +} + +class ModuleFileCache { +public: + ModuleFileCache(const GlobalCompilationDatabase &CDB) : CDB(CDB) {} + + llvm::Error + getOrBuildModuleFile(StringRef ModuleName, const ThreadsafeFS &TFS, + ProjectModules &MDB, + ReusablePrerequisiteModules &RequiredModules); + const GlobalCompilationDatabase &getCDB() const { return CDB; } + + std::shared_ptr + getValidModuleFile(StringRef ModuleName, ProjectModules &MDB, + const ThreadsafeFS &TFS, + PrerequisiteModules &BuiltModuleFiles); + + void add(StringRef ModuleName, std::shared_ptr ModuleFile) { +std::lock_guard _(ModuleFilesMutex); + +ModuleFiles.insert_or_assign(ModuleName, ModuleFile); + } + +private: + const GlobalCompilationDatabase &CDB; + + llvm::StringMap> ModuleFiles; + // Mutex to guard accesses to ModuleFiles. + std::mutex ModuleFilesMutex; +}; + +/// Collect the directly and indirectly required module names for \param +/// ModuleName. The \param ModuleName is guaranteed to be the first element in +/// \param ModuleNames. +void getAllRequiredModules(ProjectModules &MDB, StringRef ModuleName, + llvm::SmallVector &ModuleNames) { + std::queue Worklist; + llvm::StringSet<> ModuleNamesSet; + Worklist.push(ModuleName); + + while (!Worklist.empty()) { +StringRef CurrentModule = Worklist.front(); +Worklist.pop(); + +if (!ModuleNamesSet.insert(CurrentModule).second) + continue; + +ModuleNames.push_back(CurrentModule); + +for (StringRef RequiredModuleName : + MDB.getRequiredModules(MDB.getSourceForModuleName(CurrentModule))) + if (!ModuleNamesSet.contains(RequiredModuleName)) ChuanqiXu9 wrote: Oh, I read your comment incorrectly. I thought you're saying to insert `Worklist` directly. Done https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
@@ -124,10 +164,34 @@ struct ModuleFile { llvm::sys::fs::remove(ModuleFilePath); } + StringRef getModuleName() const { return ModuleName; } + + StringRef getModuleFilePath() const { return ModuleFilePath; } + +private: std::string ModuleName; std::string ModuleFilePath; + + // The required module files. We need to share the ownership for required + // module files. + ReusablePrerequisiteModules RequiredModuleFiles; ChuanqiXu9 wrote: Oh, sorry, I was confused. https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
@@ -316,36 +309,221 @@ llvm::Error buildModuleFile(llvm::StringRef ModuleName, if (Clang->getDiagnostics().hasErrorOccurred()) return llvm::createStringError("Compilation failed"); - BuiltModuleFiles.addModuleFile(ModuleName, Inputs.CompileCommand.Output); - return llvm::Error::success(); + return ModuleFile{ModuleName, Inputs.CompileCommand.Output, BuiltModuleFiles}; +} + +bool ReusablePrerequisiteModules::canReuse( +const CompilerInvocation &CI, +llvm::IntrusiveRefCntPtr VFS) const { + if (RequiredModules.empty()) +return true; + + SmallVector BMIPaths; + for (auto &MF : RequiredModules) +BMIPaths.push_back(MF->getModuleFilePath()); + return IsModuleFilesUpToDate(BMIPaths, *this, VFS); +} + +class ModuleFileCache { +public: + ModuleFileCache(const GlobalCompilationDatabase &CDB) : CDB(CDB) {} + + llvm::Error + getOrBuildModuleFile(StringRef ModuleName, const ThreadsafeFS &TFS, + ProjectModules &MDB, + ReusablePrerequisiteModules &RequiredModules); + const GlobalCompilationDatabase &getCDB() const { return CDB; } + + std::shared_ptr + getValidModuleFile(StringRef ModuleName, ProjectModules &MDB, + const ThreadsafeFS &TFS, + PrerequisiteModules &BuiltModuleFiles); + + void add(StringRef ModuleName, std::shared_ptr ModuleFile) { +std::lock_guard Lock(ModuleFilesMutex); + +ModuleFiles.insert_or_assign(ModuleName, ModuleFile); + } + +private: + const GlobalCompilationDatabase &CDB; + + llvm::StringMap> ModuleFiles; + // Mutex to guard accesses to ModuleFiles. + std::mutex ModuleFilesMutex; +}; + +/// Collect the directly and indirectly required module names for \param +/// ModuleName. The \param ModuleName is guaranteed to be the first element in +/// \param ModuleNames. +llvm::SmallVector getAllRequiredModules(ProjectModules &MDB, + StringRef ModuleName) { + llvm::SmallVector ModuleNames; + + std::queue Worklist; + llvm::StringSet<> ModuleNamesSet; + Worklist.push(ModuleName); + + while (!Worklist.empty()) { +StringRef CurrentModule = Worklist.front(); +Worklist.pop(); + +if (!ModuleNamesSet.insert(CurrentModule).second) + continue; + +ModuleNames.push_back(CurrentModule); + +for (StringRef RequiredModuleName : + MDB.getRequiredModules(MDB.getSourceForModuleName(CurrentModule))) + if (!ModuleNamesSet.contains(RequiredModuleName)) +Worklist.push(RequiredModuleName); + } + + return ModuleNames; +} + +std::shared_ptr +ModuleFileCache::getValidModuleFile(StringRef ModuleName, ProjectModules &MDB, +const ThreadsafeFS &TFS, +PrerequisiteModules &BuiltModuleFiles) { + { +std::lock_guard Lock(ModuleFilesMutex); + +auto Iter = ModuleFiles.find(ModuleName); +if (Iter == ModuleFiles.end()) + return nullptr; + +if (Iter->second.expired()) { + ModuleFiles.erase(Iter); + return nullptr; +} ChuanqiXu9 wrote: I wanted to avoid the following more expensive check as much as possible. But it is not a big deal. Done by dropping the check. https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
@@ -316,36 +287,169 @@ llvm::Error buildModuleFile(llvm::StringRef ModuleName, if (Clang->getDiagnostics().hasErrorOccurred()) return llvm::createStringError("Compilation failed"); - BuiltModuleFiles.addModuleFile(ModuleName, Inputs.CompileCommand.Output); - return llvm::Error::success(); + return ModuleFile{ModuleName, Inputs.CompileCommand.Output}; +} + +bool ReusablePrerequisiteModules::canReuse( +const CompilerInvocation &CI, +llvm::IntrusiveRefCntPtr VFS) const { + if (RequiredModules.empty()) +return true; + + SmallVector BMIPaths; + for (auto &MF : RequiredModules) +BMIPaths.push_back(MF->ModuleFilePath); + return IsModuleFilesUpToDate(BMIPaths, *this, VFS); } } // namespace +class ModulesBuilder::ModuleFileCache { +public: + ModuleFileCache(const GlobalCompilationDatabase &CDB) : CDB(CDB) {} + + llvm::Error + getOrBuildModuleFile(StringRef ModuleName, const ThreadsafeFS &TFS, + ProjectModules &MDB, + ReusablePrerequisiteModules &RequiredModules); + const GlobalCompilationDatabase &getCDB() const { return CDB; } + +private: + std::shared_ptr + getValidModuleFile(StringRef ModuleName, ProjectModules &MDB, + const ThreadsafeFS &TFS, + PrerequisiteModules &BuiltModuleFiles); + + /// This should only be called by getValidModuleFile. This is unlocked version + /// of getValidModuleFile. The function is extracted to avoid dead locks when + /// recursing. + std::shared_ptr + isValidModuleFileUnlocked(StringRef ModuleName, ProjectModules &MDB, +const ThreadsafeFS &TFS, +PrerequisiteModules &BuiltModuleFiles); + + const GlobalCompilationDatabase &CDB; + + llvm::StringMap> ModuleFiles; kadircet wrote: is this an issue you've seen in practice or just a thought experiment ? I was also mentioning this on the change itself, but in such a scenario every TU doesn't just own a `module` they own a `PrerequisiteModules` object, which underneath keeps a handle to all the required modules. Hence in your example TU `D` should keep both module `A` and `B` alive. https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
@@ -316,36 +309,221 @@ llvm::Error buildModuleFile(llvm::StringRef ModuleName, if (Clang->getDiagnostics().hasErrorOccurred()) return llvm::createStringError("Compilation failed"); - BuiltModuleFiles.addModuleFile(ModuleName, Inputs.CompileCommand.Output); - return llvm::Error::success(); + return ModuleFile{ModuleName, Inputs.CompileCommand.Output, BuiltModuleFiles}; +} + +bool ReusablePrerequisiteModules::canReuse( +const CompilerInvocation &CI, +llvm::IntrusiveRefCntPtr VFS) const { + if (RequiredModules.empty()) +return true; + + SmallVector BMIPaths; + for (auto &MF : RequiredModules) +BMIPaths.push_back(MF->getModuleFilePath()); + return IsModuleFilesUpToDate(BMIPaths, *this, VFS); +} + +class ModuleFileCache { +public: + ModuleFileCache(const GlobalCompilationDatabase &CDB) : CDB(CDB) {} + + llvm::Error + getOrBuildModuleFile(StringRef ModuleName, const ThreadsafeFS &TFS, + ProjectModules &MDB, + ReusablePrerequisiteModules &RequiredModules); + const GlobalCompilationDatabase &getCDB() const { return CDB; } + + std::shared_ptr + getValidModuleFile(StringRef ModuleName, ProjectModules &MDB, + const ThreadsafeFS &TFS, + PrerequisiteModules &BuiltModuleFiles); + + void add(StringRef ModuleName, std::shared_ptr ModuleFile) { +std::lock_guard Lock(ModuleFilesMutex); + +ModuleFiles.insert_or_assign(ModuleName, ModuleFile); + } + +private: + const GlobalCompilationDatabase &CDB; + + llvm::StringMap> ModuleFiles; + // Mutex to guard accesses to ModuleFiles. + std::mutex ModuleFilesMutex; +}; + +/// Collect the directly and indirectly required module names for \param +/// ModuleName. The \param ModuleName is guaranteed to be the first element in +/// \param ModuleNames. +llvm::SmallVector getAllRequiredModules(ProjectModules &MDB, + StringRef ModuleName) { + llvm::SmallVector ModuleNames; + + std::queue Worklist; + llvm::StringSet<> ModuleNamesSet; + Worklist.push(ModuleName); + + while (!Worklist.empty()) { +StringRef CurrentModule = Worklist.front(); +Worklist.pop(); + +if (!ModuleNamesSet.insert(CurrentModule).second) + continue; + +ModuleNames.push_back(CurrentModule); + +for (StringRef RequiredModuleName : + MDB.getRequiredModules(MDB.getSourceForModuleName(CurrentModule))) + if (!ModuleNamesSet.contains(RequiredModuleName)) +Worklist.push(RequiredModuleName); + } + + return ModuleNames; +} + +std::shared_ptr +ModuleFileCache::getValidModuleFile(StringRef ModuleName, ProjectModules &MDB, +const ThreadsafeFS &TFS, +PrerequisiteModules &BuiltModuleFiles) { + { +std::lock_guard Lock(ModuleFilesMutex); + +auto Iter = ModuleFiles.find(ModuleName); +if (Iter == ModuleFiles.end()) + return nullptr; + +if (Iter->second.expired()) { + ModuleFiles.erase(Iter); + return nullptr; +} kadircet wrote: no point in performing this check now. it can expire until we perform the loop below and we're already bailing out if this is the case. so either drop this check, or put a copy into `RequiredModuleFiles` after lookup (and change `getAllRequiredModules` to not return the root module) https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
@@ -316,36 +309,221 @@ llvm::Error buildModuleFile(llvm::StringRef ModuleName, if (Clang->getDiagnostics().hasErrorOccurred()) return llvm::createStringError("Compilation failed"); - BuiltModuleFiles.addModuleFile(ModuleName, Inputs.CompileCommand.Output); - return llvm::Error::success(); + return ModuleFile{ModuleName, Inputs.CompileCommand.Output, BuiltModuleFiles}; +} + +bool ReusablePrerequisiteModules::canReuse( +const CompilerInvocation &CI, +llvm::IntrusiveRefCntPtr VFS) const { + if (RequiredModules.empty()) +return true; + + SmallVector BMIPaths; + for (auto &MF : RequiredModules) +BMIPaths.push_back(MF->getModuleFilePath()); + return IsModuleFilesUpToDate(BMIPaths, *this, VFS); +} + +class ModuleFileCache { +public: + ModuleFileCache(const GlobalCompilationDatabase &CDB) : CDB(CDB) {} + + llvm::Error + getOrBuildModuleFile(StringRef ModuleName, const ThreadsafeFS &TFS, + ProjectModules &MDB, + ReusablePrerequisiteModules &RequiredModules); + const GlobalCompilationDatabase &getCDB() const { return CDB; } + + std::shared_ptr + getValidModuleFile(StringRef ModuleName, ProjectModules &MDB, + const ThreadsafeFS &TFS, + PrerequisiteModules &BuiltModuleFiles); + + void add(StringRef ModuleName, std::shared_ptr ModuleFile) { +std::lock_guard Lock(ModuleFilesMutex); + +ModuleFiles.insert_or_assign(ModuleName, ModuleFile); + } + +private: + const GlobalCompilationDatabase &CDB; + + llvm::StringMap> ModuleFiles; + // Mutex to guard accesses to ModuleFiles. + std::mutex ModuleFilesMutex; +}; + +/// Collect the directly and indirectly required module names for \param +/// ModuleName. The \param ModuleName is guaranteed to be the first element in +/// \param ModuleNames. +llvm::SmallVector getAllRequiredModules(ProjectModules &MDB, + StringRef ModuleName) { + llvm::SmallVector ModuleNames; + + std::queue Worklist; + llvm::StringSet<> ModuleNamesSet; + Worklist.push(ModuleName); + + while (!Worklist.empty()) { +StringRef CurrentModule = Worklist.front(); +Worklist.pop(); + +if (!ModuleNamesSet.insert(CurrentModule).second) + continue; + +ModuleNames.push_back(CurrentModule); + +for (StringRef RequiredModuleName : + MDB.getRequiredModules(MDB.getSourceForModuleName(CurrentModule))) + if (!ModuleNamesSet.contains(RequiredModuleName)) +Worklist.push(RequiredModuleName); + } + + return ModuleNames; +} + +std::shared_ptr +ModuleFileCache::getValidModuleFile(StringRef ModuleName, ProjectModules &MDB, +const ThreadsafeFS &TFS, +PrerequisiteModules &BuiltModuleFiles) { + { +std::lock_guard Lock(ModuleFilesMutex); + +auto Iter = ModuleFiles.find(ModuleName); +if (Iter == ModuleFiles.end()) + return nullptr; + +if (Iter->second.expired()) { + ModuleFiles.erase(Iter); + return nullptr; +} + } + + llvm::SmallVector ModuleNames = + getAllRequiredModules(MDB, ModuleName); + + llvm::SmallVector> RequiredModuleFiles; + + { +std::lock_guard Lock(ModuleFilesMutex); + +for (StringRef ModuleName : ModuleNames) { + auto Iter = ModuleFiles.find(ModuleName); + if (Iter == ModuleFiles.end()) +return nullptr; + + if (Iter->second.expired()) { +ModuleFiles.erase(Iter); +return nullptr; + } + + RequiredModuleFiles.push_back(Iter->second.lock()); +} + } + + assert(!RequiredModuleFiles.empty()); + + if (llvm::any_of(RequiredModuleFiles, + [&](std::shared_ptr MF) { + return !IsModuleFileUpToDate(MF->getModuleFilePath(), + BuiltModuleFiles, + TFS.view(std::nullopt)); + })) +return nullptr; + + return RequiredModuleFiles[0]; } } // namespace +class ModulesBuilder::ModulesBuilderImpl { +public: + ModulesBuilderImpl(const GlobalCompilationDatabase &CDB) : Cache(CDB) {} + + const GlobalCompilationDatabase &getCDB() const { return Cache.getCDB(); } + + llvm::Error + getOrBuildModuleFile(StringRef ModuleName, const ThreadsafeFS &TFS, + ProjectModules &MDB, + ReusablePrerequisiteModules &BuiltModuleFiles); + +private: + ModuleFileCache Cache; +}; + +llvm::Error ModulesBuilder::ModulesBuilderImpl::getOrBuildModuleFile( +StringRef ModuleName, const ThreadsafeFS &TFS, ProjectModules &MDB, +ReusablePrerequisiteModules &BuiltModuleFiles) { + if (BuiltModuleFiles.isModuleUnitBuilt(ModuleName)) +return llvm::Error::success(); + + PathRef ModuleUnitFileName = MDB.getSourceForModuleName(ModuleName); + /// It is possible that we're meeting third party modules (modules whose + /// source are not in the
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
https://github.com/kadircet requested changes to this pull request. https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
@@ -316,36 +294,205 @@ llvm::Error buildModuleFile(llvm::StringRef ModuleName, if (Clang->getDiagnostics().hasErrorOccurred()) return llvm::createStringError("Compilation failed"); - BuiltModuleFiles.addModuleFile(ModuleName, Inputs.CompileCommand.Output); - return llvm::Error::success(); + return ModuleFile{ModuleName, Inputs.CompileCommand.Output}; +} + +bool ReusablePrerequisiteModules::canReuse( +const CompilerInvocation &CI, +llvm::IntrusiveRefCntPtr VFS) const { + if (RequiredModules.empty()) +return true; + + SmallVector BMIPaths; + for (auto &MF : RequiredModules) +BMIPaths.push_back(MF->getModuleFilePath()); + return IsModuleFilesUpToDate(BMIPaths, *this, VFS); +} + +class ModuleFileCache { +public: + ModuleFileCache(const GlobalCompilationDatabase &CDB) : CDB(CDB) {} + + llvm::Error + getOrBuildModuleFile(StringRef ModuleName, const ThreadsafeFS &TFS, + ProjectModules &MDB, + ReusablePrerequisiteModules &RequiredModules); + const GlobalCompilationDatabase &getCDB() const { return CDB; } + + std::shared_ptr + getValidModuleFile(StringRef ModuleName, ProjectModules &MDB, + const ThreadsafeFS &TFS, + PrerequisiteModules &BuiltModuleFiles); + + void add(StringRef ModuleName, std::shared_ptr ModuleFile) { +std::lock_guard _(ModuleFilesMutex); + +ModuleFiles.insert_or_assign(ModuleName, ModuleFile); + } + +private: + const GlobalCompilationDatabase &CDB; + + llvm::StringMap> ModuleFiles; + // Mutex to guard accesses to ModuleFiles. + std::mutex ModuleFilesMutex; +}; + +/// Collect the directly and indirectly required module names for \param +/// ModuleName. The \param ModuleName is guaranteed to be the first element in +/// \param ModuleNames. +void getAllRequiredModules(ProjectModules &MDB, StringRef ModuleName, + llvm::SmallVector &ModuleNames) { + std::queue Worklist; + llvm::StringSet<> ModuleNamesSet; + Worklist.push(ModuleName); + + while (!Worklist.empty()) { +StringRef CurrentModule = Worklist.front(); +Worklist.pop(); + +if (!ModuleNamesSet.insert(CurrentModule).second) + continue; + +ModuleNames.push_back(CurrentModule); + +for (StringRef RequiredModuleName : + MDB.getRequiredModules(MDB.getSourceForModuleName(CurrentModule))) + if (!ModuleNamesSet.contains(RequiredModuleName)) kadircet wrote: you're checking if `ModuleNamesSet` contains an item, but inserting into `Worklist`. as a result, you'll have duplicate module-names in `Worklist` until you process them. hence I am suggesting `ModuleNamesSet.insert(RequiredModuleName)` here instead, to make sure we don't have duplicates in the queue. https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
@@ -124,10 +164,34 @@ struct ModuleFile { llvm::sys::fs::remove(ModuleFilePath); } + StringRef getModuleName() const { return ModuleName; } + + StringRef getModuleFilePath() const { return ModuleFilePath; } + +private: std::string ModuleName; std::string ModuleFilePath; + + // The required module files. We need to share the ownership for required + // module files. + ReusablePrerequisiteModules RequiredModuleFiles; kadircet wrote: why? since `ModuleFile`s are kept alive by `ReusablePrerequisiteModules`, all of their dependencies should be alive as well, no? https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
@@ -316,36 +294,205 @@ llvm::Error buildModuleFile(llvm::StringRef ModuleName, if (Clang->getDiagnostics().hasErrorOccurred()) return llvm::createStringError("Compilation failed"); - BuiltModuleFiles.addModuleFile(ModuleName, Inputs.CompileCommand.Output); - return llvm::Error::success(); + return ModuleFile{ModuleName, Inputs.CompileCommand.Output}; +} + +bool ReusablePrerequisiteModules::canReuse( +const CompilerInvocation &CI, +llvm::IntrusiveRefCntPtr VFS) const { + if (RequiredModules.empty()) +return true; + + SmallVector BMIPaths; + for (auto &MF : RequiredModules) +BMIPaths.push_back(MF->getModuleFilePath()); + return IsModuleFilesUpToDate(BMIPaths, *this, VFS); +} + +class ModuleFileCache { +public: + ModuleFileCache(const GlobalCompilationDatabase &CDB) : CDB(CDB) {} + + llvm::Error + getOrBuildModuleFile(StringRef ModuleName, const ThreadsafeFS &TFS, + ProjectModules &MDB, + ReusablePrerequisiteModules &RequiredModules); + const GlobalCompilationDatabase &getCDB() const { return CDB; } + + std::shared_ptr + getValidModuleFile(StringRef ModuleName, ProjectModules &MDB, + const ThreadsafeFS &TFS, + PrerequisiteModules &BuiltModuleFiles); + + void add(StringRef ModuleName, std::shared_ptr ModuleFile) { +std::lock_guard _(ModuleFilesMutex); + +ModuleFiles.insert_or_assign(ModuleName, ModuleFile); + } + +private: + const GlobalCompilationDatabase &CDB; + + llvm::StringMap> ModuleFiles; + // Mutex to guard accesses to ModuleFiles. + std::mutex ModuleFilesMutex; +}; + +/// Collect the directly and indirectly required module names for \param +/// ModuleName. The \param ModuleName is guaranteed to be the first element in +/// \param ModuleNames. +void getAllRequiredModules(ProjectModules &MDB, StringRef ModuleName, + llvm::SmallVector &ModuleNames) { + std::queue Worklist; + llvm::StringSet<> ModuleNamesSet; + Worklist.push(ModuleName); + + while (!Worklist.empty()) { +StringRef CurrentModule = Worklist.front(); +Worklist.pop(); + +if (!ModuleNamesSet.insert(CurrentModule).second) + continue; + +ModuleNames.push_back(CurrentModule); + +for (StringRef RequiredModuleName : + MDB.getRequiredModules(MDB.getSourceForModuleName(CurrentModule))) + if (!ModuleNamesSet.contains(RequiredModuleName)) +Worklist.push(RequiredModuleName); + } +} + +std::shared_ptr +ModuleFileCache::getValidModuleFile(StringRef ModuleName, ProjectModules &MDB, +const ThreadsafeFS &TFS, +PrerequisiteModules &BuiltModuleFiles) { + { +std::lock_guard _(ModuleFilesMutex); + +if (ModuleFiles.find(ModuleName) == ModuleFiles.end()) + return nullptr; + } + + llvm::SmallVector ModuleNames; + getAllRequiredModules(MDB, ModuleName, ModuleNames); + + llvm::SmallVector> RequiredModuleFiles; + + { +std::lock_guard _(ModuleFilesMutex); + +for (StringRef ModuleName : ModuleNames) { + auto Iter = ModuleFiles.find(ModuleName); + if (Iter == ModuleFiles.end()) +return nullptr; + + RequiredModuleFiles.push_back(Iter->second); +} + } + + if (RequiredModuleFiles.empty()) +return nullptr; kadircet wrote: if that's what you're worried about, i think we should be asserting on `ModuleNames` not being empty instead. https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
https://github.com/ChuanqiXu9 edited https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
@@ -316,36 +294,205 @@ llvm::Error buildModuleFile(llvm::StringRef ModuleName, if (Clang->getDiagnostics().hasErrorOccurred()) return llvm::createStringError("Compilation failed"); - BuiltModuleFiles.addModuleFile(ModuleName, Inputs.CompileCommand.Output); - return llvm::Error::success(); + return ModuleFile{ModuleName, Inputs.CompileCommand.Output}; +} + +bool ReusablePrerequisiteModules::canReuse( +const CompilerInvocation &CI, +llvm::IntrusiveRefCntPtr VFS) const { + if (RequiredModules.empty()) +return true; + + SmallVector BMIPaths; + for (auto &MF : RequiredModules) +BMIPaths.push_back(MF->getModuleFilePath()); + return IsModuleFilesUpToDate(BMIPaths, *this, VFS); +} + +class ModuleFileCache { +public: + ModuleFileCache(const GlobalCompilationDatabase &CDB) : CDB(CDB) {} + + llvm::Error + getOrBuildModuleFile(StringRef ModuleName, const ThreadsafeFS &TFS, + ProjectModules &MDB, + ReusablePrerequisiteModules &RequiredModules); + const GlobalCompilationDatabase &getCDB() const { return CDB; } + + std::shared_ptr + getValidModuleFile(StringRef ModuleName, ProjectModules &MDB, + const ThreadsafeFS &TFS, + PrerequisiteModules &BuiltModuleFiles); + + void add(StringRef ModuleName, std::shared_ptr ModuleFile) { +std::lock_guard _(ModuleFilesMutex); + +ModuleFiles.insert_or_assign(ModuleName, ModuleFile); + } + +private: + const GlobalCompilationDatabase &CDB; + + llvm::StringMap> ModuleFiles; + // Mutex to guard accesses to ModuleFiles. + std::mutex ModuleFilesMutex; +}; + +/// Collect the directly and indirectly required module names for \param +/// ModuleName. The \param ModuleName is guaranteed to be the first element in +/// \param ModuleNames. +void getAllRequiredModules(ProjectModules &MDB, StringRef ModuleName, + llvm::SmallVector &ModuleNames) { + std::queue Worklist; + llvm::StringSet<> ModuleNamesSet; + Worklist.push(ModuleName); + + while (!Worklist.empty()) { +StringRef CurrentModule = Worklist.front(); +Worklist.pop(); + +if (!ModuleNamesSet.insert(CurrentModule).second) + continue; + +ModuleNames.push_back(CurrentModule); + +for (StringRef RequiredModuleName : + MDB.getRequiredModules(MDB.getSourceForModuleName(CurrentModule))) + if (!ModuleNamesSet.contains(RequiredModuleName)) +Worklist.push(RequiredModuleName); + } +} + +std::shared_ptr +ModuleFileCache::getValidModuleFile(StringRef ModuleName, ProjectModules &MDB, +const ThreadsafeFS &TFS, +PrerequisiteModules &BuiltModuleFiles) { + { +std::lock_guard _(ModuleFilesMutex); + +if (ModuleFiles.find(ModuleName) == ModuleFiles.end()) + return nullptr; + } + + llvm::SmallVector ModuleNames; + getAllRequiredModules(MDB, ModuleName, ModuleNames); + + llvm::SmallVector> RequiredModuleFiles; + + { +std::lock_guard _(ModuleFilesMutex); + +for (StringRef ModuleName : ModuleNames) { + auto Iter = ModuleFiles.find(ModuleName); + if (Iter == ModuleFiles.end()) +return nullptr; + + RequiredModuleFiles.push_back(Iter->second); +} + } + + if (RequiredModuleFiles.empty()) +return nullptr; ChuanqiXu9 wrote: I changed it to an assertion. It will be helpful if the implementation `getAllRequiredModules` gets wrong someday. https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
@@ -316,36 +294,205 @@ llvm::Error buildModuleFile(llvm::StringRef ModuleName, if (Clang->getDiagnostics().hasErrorOccurred()) return llvm::createStringError("Compilation failed"); - BuiltModuleFiles.addModuleFile(ModuleName, Inputs.CompileCommand.Output); - return llvm::Error::success(); + return ModuleFile{ModuleName, Inputs.CompileCommand.Output}; +} + +bool ReusablePrerequisiteModules::canReuse( +const CompilerInvocation &CI, +llvm::IntrusiveRefCntPtr VFS) const { + if (RequiredModules.empty()) +return true; + + SmallVector BMIPaths; + for (auto &MF : RequiredModules) +BMIPaths.push_back(MF->getModuleFilePath()); + return IsModuleFilesUpToDate(BMIPaths, *this, VFS); +} + +class ModuleFileCache { +public: + ModuleFileCache(const GlobalCompilationDatabase &CDB) : CDB(CDB) {} + + llvm::Error + getOrBuildModuleFile(StringRef ModuleName, const ThreadsafeFS &TFS, + ProjectModules &MDB, + ReusablePrerequisiteModules &RequiredModules); + const GlobalCompilationDatabase &getCDB() const { return CDB; } + + std::shared_ptr + getValidModuleFile(StringRef ModuleName, ProjectModules &MDB, + const ThreadsafeFS &TFS, + PrerequisiteModules &BuiltModuleFiles); + + void add(StringRef ModuleName, std::shared_ptr ModuleFile) { +std::lock_guard _(ModuleFilesMutex); + +ModuleFiles.insert_or_assign(ModuleName, ModuleFile); + } + +private: + const GlobalCompilationDatabase &CDB; + + llvm::StringMap> ModuleFiles; + // Mutex to guard accesses to ModuleFiles. + std::mutex ModuleFilesMutex; +}; + +/// Collect the directly and indirectly required module names for \param +/// ModuleName. The \param ModuleName is guaranteed to be the first element in +/// \param ModuleNames. +void getAllRequiredModules(ProjectModules &MDB, StringRef ModuleName, + llvm::SmallVector &ModuleNames) { + std::queue Worklist; + llvm::StringSet<> ModuleNamesSet; + Worklist.push(ModuleName); + + while (!Worklist.empty()) { +StringRef CurrentModule = Worklist.front(); +Worklist.pop(); + +if (!ModuleNamesSet.insert(CurrentModule).second) + continue; + +ModuleNames.push_back(CurrentModule); + +for (StringRef RequiredModuleName : + MDB.getRequiredModules(MDB.getSourceForModuleName(CurrentModule))) + if (!ModuleNamesSet.contains(RequiredModuleName)) +Worklist.push(RequiredModuleName); + } +} + +std::shared_ptr +ModuleFileCache::getValidModuleFile(StringRef ModuleName, ProjectModules &MDB, +const ThreadsafeFS &TFS, +PrerequisiteModules &BuiltModuleFiles) { + { +std::lock_guard _(ModuleFilesMutex); + +if (ModuleFiles.find(ModuleName) == ModuleFiles.end()) + return nullptr; + } + + llvm::SmallVector ModuleNames; + getAllRequiredModules(MDB, ModuleName, ModuleNames); + + llvm::SmallVector> RequiredModuleFiles; + + { +std::lock_guard _(ModuleFilesMutex); + +for (StringRef ModuleName : ModuleNames) { + auto Iter = ModuleFiles.find(ModuleName); + if (Iter == ModuleFiles.end()) +return nullptr; + + RequiredModuleFiles.push_back(Iter->second); +} + } + + if (RequiredModuleFiles.empty()) +return nullptr; + + if (llvm::all_of(RequiredModuleFiles, [&](auto MF) { ChuanqiXu9 wrote: Done https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
@@ -316,36 +294,205 @@ llvm::Error buildModuleFile(llvm::StringRef ModuleName, if (Clang->getDiagnostics().hasErrorOccurred()) return llvm::createStringError("Compilation failed"); - BuiltModuleFiles.addModuleFile(ModuleName, Inputs.CompileCommand.Output); - return llvm::Error::success(); + return ModuleFile{ModuleName, Inputs.CompileCommand.Output}; +} + +bool ReusablePrerequisiteModules::canReuse( +const CompilerInvocation &CI, +llvm::IntrusiveRefCntPtr VFS) const { + if (RequiredModules.empty()) +return true; + + SmallVector BMIPaths; + for (auto &MF : RequiredModules) +BMIPaths.push_back(MF->getModuleFilePath()); + return IsModuleFilesUpToDate(BMIPaths, *this, VFS); +} + +class ModuleFileCache { +public: + ModuleFileCache(const GlobalCompilationDatabase &CDB) : CDB(CDB) {} + + llvm::Error + getOrBuildModuleFile(StringRef ModuleName, const ThreadsafeFS &TFS, + ProjectModules &MDB, + ReusablePrerequisiteModules &RequiredModules); + const GlobalCompilationDatabase &getCDB() const { return CDB; } + + std::shared_ptr + getValidModuleFile(StringRef ModuleName, ProjectModules &MDB, + const ThreadsafeFS &TFS, + PrerequisiteModules &BuiltModuleFiles); + + void add(StringRef ModuleName, std::shared_ptr ModuleFile) { +std::lock_guard _(ModuleFilesMutex); + +ModuleFiles.insert_or_assign(ModuleName, ModuleFile); + } + +private: + const GlobalCompilationDatabase &CDB; + + llvm::StringMap> ModuleFiles; + // Mutex to guard accesses to ModuleFiles. + std::mutex ModuleFilesMutex; +}; + +/// Collect the directly and indirectly required module names for \param +/// ModuleName. The \param ModuleName is guaranteed to be the first element in +/// \param ModuleNames. +void getAllRequiredModules(ProjectModules &MDB, StringRef ModuleName, + llvm::SmallVector &ModuleNames) { + std::queue Worklist; + llvm::StringSet<> ModuleNamesSet; + Worklist.push(ModuleName); + + while (!Worklist.empty()) { +StringRef CurrentModule = Worklist.front(); +Worklist.pop(); + +if (!ModuleNamesSet.insert(CurrentModule).second) + continue; + +ModuleNames.push_back(CurrentModule); + +for (StringRef RequiredModuleName : + MDB.getRequiredModules(MDB.getSourceForModuleName(CurrentModule))) + if (!ModuleNamesSet.contains(RequiredModuleName)) +Worklist.push(RequiredModuleName); + } +} + +std::shared_ptr +ModuleFileCache::getValidModuleFile(StringRef ModuleName, ProjectModules &MDB, +const ThreadsafeFS &TFS, +PrerequisiteModules &BuiltModuleFiles) { + { +std::lock_guard _(ModuleFilesMutex); + +if (ModuleFiles.find(ModuleName) == ModuleFiles.end()) + return nullptr; + } + + llvm::SmallVector ModuleNames; + getAllRequiredModules(MDB, ModuleName, ModuleNames); + + llvm::SmallVector> RequiredModuleFiles; + + { +std::lock_guard _(ModuleFilesMutex); + +for (StringRef ModuleName : ModuleNames) { + auto Iter = ModuleFiles.find(ModuleName); + if (Iter == ModuleFiles.end()) +return nullptr; + + RequiredModuleFiles.push_back(Iter->second); +} + } + + if (RequiredModuleFiles.empty()) +return nullptr; + + if (llvm::all_of(RequiredModuleFiles, [&](auto MF) { ChuanqiXu9 wrote: Done https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
@@ -316,36 +294,205 @@ llvm::Error buildModuleFile(llvm::StringRef ModuleName, if (Clang->getDiagnostics().hasErrorOccurred()) return llvm::createStringError("Compilation failed"); - BuiltModuleFiles.addModuleFile(ModuleName, Inputs.CompileCommand.Output); - return llvm::Error::success(); + return ModuleFile{ModuleName, Inputs.CompileCommand.Output}; +} + +bool ReusablePrerequisiteModules::canReuse( +const CompilerInvocation &CI, +llvm::IntrusiveRefCntPtr VFS) const { + if (RequiredModules.empty()) +return true; + + SmallVector BMIPaths; + for (auto &MF : RequiredModules) +BMIPaths.push_back(MF->getModuleFilePath()); + return IsModuleFilesUpToDate(BMIPaths, *this, VFS); +} + +class ModuleFileCache { +public: + ModuleFileCache(const GlobalCompilationDatabase &CDB) : CDB(CDB) {} + + llvm::Error + getOrBuildModuleFile(StringRef ModuleName, const ThreadsafeFS &TFS, + ProjectModules &MDB, + ReusablePrerequisiteModules &RequiredModules); + const GlobalCompilationDatabase &getCDB() const { return CDB; } + + std::shared_ptr + getValidModuleFile(StringRef ModuleName, ProjectModules &MDB, + const ThreadsafeFS &TFS, + PrerequisiteModules &BuiltModuleFiles); + + void add(StringRef ModuleName, std::shared_ptr ModuleFile) { +std::lock_guard _(ModuleFilesMutex); + +ModuleFiles.insert_or_assign(ModuleName, ModuleFile); + } + +private: + const GlobalCompilationDatabase &CDB; + + llvm::StringMap> ModuleFiles; + // Mutex to guard accesses to ModuleFiles. + std::mutex ModuleFilesMutex; +}; + +/// Collect the directly and indirectly required module names for \param +/// ModuleName. The \param ModuleName is guaranteed to be the first element in +/// \param ModuleNames. +void getAllRequiredModules(ProjectModules &MDB, StringRef ModuleName, + llvm::SmallVector &ModuleNames) { + std::queue Worklist; + llvm::StringSet<> ModuleNamesSet; + Worklist.push(ModuleName); + + while (!Worklist.empty()) { +StringRef CurrentModule = Worklist.front(); +Worklist.pop(); + +if (!ModuleNamesSet.insert(CurrentModule).second) + continue; + +ModuleNames.push_back(CurrentModule); + +for (StringRef RequiredModuleName : + MDB.getRequiredModules(MDB.getSourceForModuleName(CurrentModule))) + if (!ModuleNamesSet.contains(RequiredModuleName)) +Worklist.push(RequiredModuleName); + } +} + +std::shared_ptr +ModuleFileCache::getValidModuleFile(StringRef ModuleName, ProjectModules &MDB, +const ThreadsafeFS &TFS, +PrerequisiteModules &BuiltModuleFiles) { + { +std::lock_guard _(ModuleFilesMutex); + +if (ModuleFiles.find(ModuleName) == ModuleFiles.end()) ChuanqiXu9 wrote: I changed it to use `Iter` and we can erase the invalid `Iter` now. So this comment may be out-of-date. https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
@@ -316,36 +294,205 @@ llvm::Error buildModuleFile(llvm::StringRef ModuleName, if (Clang->getDiagnostics().hasErrorOccurred()) return llvm::createStringError("Compilation failed"); - BuiltModuleFiles.addModuleFile(ModuleName, Inputs.CompileCommand.Output); - return llvm::Error::success(); + return ModuleFile{ModuleName, Inputs.CompileCommand.Output}; +} + +bool ReusablePrerequisiteModules::canReuse( +const CompilerInvocation &CI, +llvm::IntrusiveRefCntPtr VFS) const { + if (RequiredModules.empty()) +return true; + + SmallVector BMIPaths; + for (auto &MF : RequiredModules) +BMIPaths.push_back(MF->getModuleFilePath()); + return IsModuleFilesUpToDate(BMIPaths, *this, VFS); +} + +class ModuleFileCache { +public: + ModuleFileCache(const GlobalCompilationDatabase &CDB) : CDB(CDB) {} + + llvm::Error + getOrBuildModuleFile(StringRef ModuleName, const ThreadsafeFS &TFS, + ProjectModules &MDB, + ReusablePrerequisiteModules &RequiredModules); + const GlobalCompilationDatabase &getCDB() const { return CDB; } + + std::shared_ptr + getValidModuleFile(StringRef ModuleName, ProjectModules &MDB, + const ThreadsafeFS &TFS, + PrerequisiteModules &BuiltModuleFiles); + + void add(StringRef ModuleName, std::shared_ptr ModuleFile) { +std::lock_guard _(ModuleFilesMutex); + +ModuleFiles.insert_or_assign(ModuleName, ModuleFile); + } + +private: + const GlobalCompilationDatabase &CDB; + + llvm::StringMap> ModuleFiles; + // Mutex to guard accesses to ModuleFiles. + std::mutex ModuleFilesMutex; +}; + +/// Collect the directly and indirectly required module names for \param +/// ModuleName. The \param ModuleName is guaranteed to be the first element in +/// \param ModuleNames. +void getAllRequiredModules(ProjectModules &MDB, StringRef ModuleName, + llvm::SmallVector &ModuleNames) { + std::queue Worklist; + llvm::StringSet<> ModuleNamesSet; + Worklist.push(ModuleName); + + while (!Worklist.empty()) { +StringRef CurrentModule = Worklist.front(); +Worklist.pop(); + +if (!ModuleNamesSet.insert(CurrentModule).second) + continue; + +ModuleNames.push_back(CurrentModule); + +for (StringRef RequiredModuleName : + MDB.getRequiredModules(MDB.getSourceForModuleName(CurrentModule))) + if (!ModuleNamesSet.contains(RequiredModuleName)) ChuanqiXu9 wrote: I can't understand. I think it should be better to avoid the redundant insert as much as possible. https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
@@ -316,36 +294,205 @@ llvm::Error buildModuleFile(llvm::StringRef ModuleName, if (Clang->getDiagnostics().hasErrorOccurred()) return llvm::createStringError("Compilation failed"); - BuiltModuleFiles.addModuleFile(ModuleName, Inputs.CompileCommand.Output); - return llvm::Error::success(); + return ModuleFile{ModuleName, Inputs.CompileCommand.Output}; +} + +bool ReusablePrerequisiteModules::canReuse( +const CompilerInvocation &CI, +llvm::IntrusiveRefCntPtr VFS) const { + if (RequiredModules.empty()) +return true; + + SmallVector BMIPaths; + for (auto &MF : RequiredModules) +BMIPaths.push_back(MF->getModuleFilePath()); + return IsModuleFilesUpToDate(BMIPaths, *this, VFS); +} + +class ModuleFileCache { +public: + ModuleFileCache(const GlobalCompilationDatabase &CDB) : CDB(CDB) {} + + llvm::Error + getOrBuildModuleFile(StringRef ModuleName, const ThreadsafeFS &TFS, + ProjectModules &MDB, + ReusablePrerequisiteModules &RequiredModules); + const GlobalCompilationDatabase &getCDB() const { return CDB; } + + std::shared_ptr + getValidModuleFile(StringRef ModuleName, ProjectModules &MDB, + const ThreadsafeFS &TFS, + PrerequisiteModules &BuiltModuleFiles); + + void add(StringRef ModuleName, std::shared_ptr ModuleFile) { +std::lock_guard _(ModuleFilesMutex); + +ModuleFiles.insert_or_assign(ModuleName, ModuleFile); + } + +private: + const GlobalCompilationDatabase &CDB; + + llvm::StringMap> ModuleFiles; + // Mutex to guard accesses to ModuleFiles. + std::mutex ModuleFilesMutex; +}; + +/// Collect the directly and indirectly required module names for \param +/// ModuleName. The \param ModuleName is guaranteed to be the first element in +/// \param ModuleNames. +void getAllRequiredModules(ProjectModules &MDB, StringRef ModuleName, ChuanqiXu9 wrote: Done https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
@@ -316,36 +287,169 @@ llvm::Error buildModuleFile(llvm::StringRef ModuleName, if (Clang->getDiagnostics().hasErrorOccurred()) return llvm::createStringError("Compilation failed"); - BuiltModuleFiles.addModuleFile(ModuleName, Inputs.CompileCommand.Output); - return llvm::Error::success(); + return ModuleFile{ModuleName, Inputs.CompileCommand.Output}; +} + +bool ReusablePrerequisiteModules::canReuse( +const CompilerInvocation &CI, +llvm::IntrusiveRefCntPtr VFS) const { + if (RequiredModules.empty()) +return true; + + SmallVector BMIPaths; + for (auto &MF : RequiredModules) +BMIPaths.push_back(MF->ModuleFilePath); + return IsModuleFilesUpToDate(BMIPaths, *this, VFS); } } // namespace +class ModulesBuilder::ModuleFileCache { +public: + ModuleFileCache(const GlobalCompilationDatabase &CDB) : CDB(CDB) {} + + llvm::Error + getOrBuildModuleFile(StringRef ModuleName, const ThreadsafeFS &TFS, + ProjectModules &MDB, + ReusablePrerequisiteModules &RequiredModules); + const GlobalCompilationDatabase &getCDB() const { return CDB; } + +private: + std::shared_ptr + getValidModuleFile(StringRef ModuleName, ProjectModules &MDB, + const ThreadsafeFS &TFS, + PrerequisiteModules &BuiltModuleFiles); + + /// This should only be called by getValidModuleFile. This is unlocked version + /// of getValidModuleFile. The function is extracted to avoid dead locks when + /// recursing. + std::shared_ptr + isValidModuleFileUnlocked(StringRef ModuleName, ProjectModules &MDB, +const ThreadsafeFS &TFS, +PrerequisiteModules &BuiltModuleFiles); + + const GlobalCompilationDatabase &CDB; + + llvm::StringMap> ModuleFiles; + // Mutex to guard accesses to ModuleFiles. + std::mutex ModuleFilesMutex; +}; + +std::shared_ptr +ModulesBuilder::ModuleFileCache::isValidModuleFileUnlocked( +StringRef ModuleName, ProjectModules &MDB, const ThreadsafeFS &TFS, +PrerequisiteModules &BuiltModuleFiles) { + auto Iter = ModuleFiles.find(ModuleName); + if (Iter != ModuleFiles.end()) { +if (!IsModuleFileUpToDate(Iter->second->ModuleFilePath, BuiltModuleFiles, + TFS.view(std::nullopt))) { + log("Found not-up-date module file {0} for module {1} in cache", + Iter->second->ModuleFilePath, ModuleName); + ModuleFiles.erase(Iter); + return nullptr; +} + +if (llvm::any_of( +MDB.getRequiredModules(MDB.getSourceForModuleName(ModuleName)), +[&MDB, &TFS, &BuiltModuleFiles, this](auto &&RequiredModuleName) { + return !isValidModuleFileUnlocked(RequiredModuleName, MDB, TFS, +BuiltModuleFiles); +})) { + ModuleFiles.erase(Iter); + return nullptr; +} + +return Iter->second; + } + + log("Don't find {0} in cache", ModuleName); + + return nullptr; +} + +std::shared_ptr ModulesBuilder::ModuleFileCache::getValidModuleFile( +StringRef ModuleName, ProjectModules &MDB, const ThreadsafeFS &TFS, +PrerequisiteModules &BuiltModuleFiles) { + std::lock_guard _(ModuleFilesMutex); + + return isValidModuleFileUnlocked(ModuleName, MDB, TFS, BuiltModuleFiles); +} + +llvm::Error ModulesBuilder::ModuleFileCache::getOrBuildModuleFile( ChuanqiXu9 wrote: I understand your intention. But I do think it is not good to merge them. Since `buildModuleFile` and `ModulesBuilderImpl ` are doing different things to me. `buildModuleFile` is doing the concrete, low-level things to build a module file. And `ModulesBuilderImpl` does the higher level things. e.g., decide if we're going to build the module file actually or when to build it. I think they are different logics. https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
https://github.com/ChuanqiXu9 edited https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
@@ -228,54 +223,30 @@ class StandalonePrerequisiteModules : public PrerequisiteModules { return BuiltModuleNames.contains(ModuleName); } - void addModuleFile(llvm::StringRef ModuleName, - llvm::StringRef ModuleFilePath) { -RequiredModules.emplace_back(ModuleName, ModuleFilePath); -BuiltModuleNames.insert(ModuleName); + void addModuleFile(std::shared_ptr BMI) { +BuiltModuleNames.insert(BMI->ModuleName); +RequiredModules.emplace_back(std::move(BMI)); } private: - llvm::SmallVector RequiredModules; + mutable llvm::SmallVector, 8> RequiredModules; ChuanqiXu9 wrote: Done https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
@@ -316,36 +287,169 @@ llvm::Error buildModuleFile(llvm::StringRef ModuleName, if (Clang->getDiagnostics().hasErrorOccurred()) return llvm::createStringError("Compilation failed"); - BuiltModuleFiles.addModuleFile(ModuleName, Inputs.CompileCommand.Output); - return llvm::Error::success(); + return ModuleFile{ModuleName, Inputs.CompileCommand.Output}; +} + +bool ReusablePrerequisiteModules::canReuse( +const CompilerInvocation &CI, +llvm::IntrusiveRefCntPtr VFS) const { + if (RequiredModules.empty()) +return true; + + SmallVector BMIPaths; + for (auto &MF : RequiredModules) +BMIPaths.push_back(MF->ModuleFilePath); + return IsModuleFilesUpToDate(BMIPaths, *this, VFS); } } // namespace +class ModulesBuilder::ModuleFileCache { +public: + ModuleFileCache(const GlobalCompilationDatabase &CDB) : CDB(CDB) {} + + llvm::Error + getOrBuildModuleFile(StringRef ModuleName, const ThreadsafeFS &TFS, + ProjectModules &MDB, + ReusablePrerequisiteModules &RequiredModules); + const GlobalCompilationDatabase &getCDB() const { return CDB; } + +private: + std::shared_ptr + getValidModuleFile(StringRef ModuleName, ProjectModules &MDB, + const ThreadsafeFS &TFS, + PrerequisiteModules &BuiltModuleFiles); + + /// This should only be called by getValidModuleFile. This is unlocked version + /// of getValidModuleFile. The function is extracted to avoid dead locks when + /// recursing. + std::shared_ptr + isValidModuleFileUnlocked(StringRef ModuleName, ProjectModules &MDB, +const ThreadsafeFS &TFS, +PrerequisiteModules &BuiltModuleFiles); + + const GlobalCompilationDatabase &CDB; + + llvm::StringMap> ModuleFiles; ChuanqiXu9 wrote: Got your points. Although I feel there are spaces to optimize, let's follow the current clangd's style. https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
https://github.com/ChuanqiXu9 updated https://github.com/llvm/llvm-project/pull/106683 >From 15aa6af1f5d22e4b837e8e2fd49469310ffbe7f1 Mon Sep 17 00:00:00 2001 From: Chuanqi Xu Date: Fri, 30 Aug 2024 15:11:07 +0800 Subject: [PATCH 1/4] [clangd] [Modules] Support Reusable Modules Builder --- clang-tools-extra/clangd/ModulesBuilder.cpp | 375 ++ clang-tools-extra/clangd/ModulesBuilder.h | 8 +- .../unittests/PrerequisiteModulesTest.cpp | 36 ++ 3 files changed, 344 insertions(+), 75 deletions(-) diff --git a/clang-tools-extra/clangd/ModulesBuilder.cpp b/clang-tools-extra/clangd/ModulesBuilder.cpp index 97f67ddf5495a2..c4fb3d4010d370 100644 --- a/clang-tools-extra/clangd/ModulesBuilder.cpp +++ b/clang-tools-extra/clangd/ModulesBuilder.cpp @@ -13,6 +13,7 @@ #include "clang/Frontend/FrontendActions.h" #include "clang/Serialization/ASTReader.h" #include "clang/Serialization/InMemoryModuleCache.h" +#include "llvm/ADT/ScopeExit.h" namespace clang { namespace clangd { @@ -192,33 +193,27 @@ bool IsModuleFilesUpToDate( }); } -// StandalonePrerequisiteModules - stands for PrerequisiteModules for which all +// ReusablePrerequisiteModules - stands for PrerequisiteModules for which all // the required modules are built successfully. All the module files -// are owned by the StandalonePrerequisiteModules class. -// -// Any of the built module files won't be shared with other instances of the -// class. So that we can avoid worrying thread safety. -// -// We don't need to worry about duplicated module names here since the standard -// guarantees the module names should be unique to a program. -class StandalonePrerequisiteModules : public PrerequisiteModules { +// are owned by the modules builder. +class ReusablePrerequisiteModules : public PrerequisiteModules { public: - StandalonePrerequisiteModules() = default; + ReusablePrerequisiteModules() = default; - StandalonePrerequisiteModules(const StandalonePrerequisiteModules &) = delete; - StandalonePrerequisiteModules - operator=(const StandalonePrerequisiteModules &) = delete; - StandalonePrerequisiteModules(StandalonePrerequisiteModules &&) = delete; - StandalonePrerequisiteModules - operator=(StandalonePrerequisiteModules &&) = delete; + ReusablePrerequisiteModules(const ReusablePrerequisiteModules &) = delete; + ReusablePrerequisiteModules + operator=(const ReusablePrerequisiteModules &) = delete; + ReusablePrerequisiteModules(ReusablePrerequisiteModules &&) = delete; + ReusablePrerequisiteModules + operator=(ReusablePrerequisiteModules &&) = delete; - ~StandalonePrerequisiteModules() override = default; + ~ReusablePrerequisiteModules() override = default; void adjustHeaderSearchOptions(HeaderSearchOptions &Options) const override { // Appending all built module files. for (auto &RequiredModule : RequiredModules) Options.PrebuiltModuleFiles.insert_or_assign( - RequiredModule.ModuleName, RequiredModule.ModuleFilePath); + RequiredModule->ModuleName, RequiredModule->ModuleFilePath); } bool canReuse(const CompilerInvocation &CI, @@ -228,54 +223,30 @@ class StandalonePrerequisiteModules : public PrerequisiteModules { return BuiltModuleNames.contains(ModuleName); } - void addModuleFile(llvm::StringRef ModuleName, - llvm::StringRef ModuleFilePath) { -RequiredModules.emplace_back(ModuleName, ModuleFilePath); -BuiltModuleNames.insert(ModuleName); + void addModuleFile(std::shared_ptr BMI) { +BuiltModuleNames.insert(BMI->ModuleName); +RequiredModules.emplace_back(std::move(BMI)); } private: - llvm::SmallVector RequiredModules; + mutable llvm::SmallVector, 8> RequiredModules; // A helper class to speedup the query if a module is built. llvm::StringSet<> BuiltModuleNames; }; -// Build a module file for module with `ModuleName`. The information of built -// module file are stored in \param BuiltModuleFiles. -llvm::Error buildModuleFile(llvm::StringRef ModuleName, -const GlobalCompilationDatabase &CDB, -const ThreadsafeFS &TFS, ProjectModules &MDB, -PathRef ModuleFilesPrefix, -StandalonePrerequisiteModules &BuiltModuleFiles) { - if (BuiltModuleFiles.isModuleUnitBuilt(ModuleName)) -return llvm::Error::success(); - - PathRef ModuleUnitFileName = MDB.getSourceForModuleName(ModuleName); - // It is possible that we're meeting third party modules (modules whose - // source are not in the project. e.g, the std module may be a third-party - // module for most projects) or something wrong with the implementation of - // ProjectModules. - // FIXME: How should we treat third party modules here? If we want to ignore - // third party modules, we should return true instead of false here. - // Currently we simply bail out. - if (ModuleUnitFileName.empty()) -return llvm::createStringErro
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
@@ -316,36 +294,205 @@ llvm::Error buildModuleFile(llvm::StringRef ModuleName, if (Clang->getDiagnostics().hasErrorOccurred()) return llvm::createStringError("Compilation failed"); - BuiltModuleFiles.addModuleFile(ModuleName, Inputs.CompileCommand.Output); - return llvm::Error::success(); + return ModuleFile{ModuleName, Inputs.CompileCommand.Output}; +} + +bool ReusablePrerequisiteModules::canReuse( +const CompilerInvocation &CI, +llvm::IntrusiveRefCntPtr VFS) const { + if (RequiredModules.empty()) +return true; + + SmallVector BMIPaths; + for (auto &MF : RequiredModules) +BMIPaths.push_back(MF->getModuleFilePath()); + return IsModuleFilesUpToDate(BMIPaths, *this, VFS); +} + +class ModuleFileCache { +public: + ModuleFileCache(const GlobalCompilationDatabase &CDB) : CDB(CDB) {} + + llvm::Error + getOrBuildModuleFile(StringRef ModuleName, const ThreadsafeFS &TFS, + ProjectModules &MDB, + ReusablePrerequisiteModules &RequiredModules); + const GlobalCompilationDatabase &getCDB() const { return CDB; } + + std::shared_ptr + getValidModuleFile(StringRef ModuleName, ProjectModules &MDB, + const ThreadsafeFS &TFS, + PrerequisiteModules &BuiltModuleFiles); + + void add(StringRef ModuleName, std::shared_ptr ModuleFile) { +std::lock_guard _(ModuleFilesMutex); + +ModuleFiles.insert_or_assign(ModuleName, ModuleFile); + } + +private: + const GlobalCompilationDatabase &CDB; + + llvm::StringMap> ModuleFiles; + // Mutex to guard accesses to ModuleFiles. + std::mutex ModuleFilesMutex; +}; + +/// Collect the directly and indirectly required module names for \param +/// ModuleName. The \param ModuleName is guaranteed to be the first element in +/// \param ModuleNames. +void getAllRequiredModules(ProjectModules &MDB, StringRef ModuleName, + llvm::SmallVector &ModuleNames) { + std::queue Worklist; + llvm::StringSet<> ModuleNamesSet; + Worklist.push(ModuleName); + + while (!Worklist.empty()) { +StringRef CurrentModule = Worklist.front(); +Worklist.pop(); + +if (!ModuleNamesSet.insert(CurrentModule).second) + continue; + +ModuleNames.push_back(CurrentModule); + +for (StringRef RequiredModuleName : + MDB.getRequiredModules(MDB.getSourceForModuleName(CurrentModule))) + if (!ModuleNamesSet.contains(RequiredModuleName)) +Worklist.push(RequiredModuleName); + } +} + +std::shared_ptr +ModuleFileCache::getValidModuleFile(StringRef ModuleName, ProjectModules &MDB, +const ThreadsafeFS &TFS, +PrerequisiteModules &BuiltModuleFiles) { + { +std::lock_guard _(ModuleFilesMutex); + +if (ModuleFiles.find(ModuleName) == ModuleFiles.end()) + return nullptr; + } + + llvm::SmallVector ModuleNames; + getAllRequiredModules(MDB, ModuleName, ModuleNames); + + llvm::SmallVector> RequiredModuleFiles; + + { +std::lock_guard _(ModuleFilesMutex); + +for (StringRef ModuleName : ModuleNames) { + auto Iter = ModuleFiles.find(ModuleName); + if (Iter == ModuleFiles.end()) +return nullptr; + + RequiredModuleFiles.push_back(Iter->second); +} + } + + if (RequiredModuleFiles.empty()) +return nullptr; + + if (llvm::all_of(RequiredModuleFiles, [&](auto MF) { kadircet wrote: s/auto/llvm::StringRef no need to introduce templates here https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
@@ -316,36 +287,169 @@ llvm::Error buildModuleFile(llvm::StringRef ModuleName, if (Clang->getDiagnostics().hasErrorOccurred()) return llvm::createStringError("Compilation failed"); - BuiltModuleFiles.addModuleFile(ModuleName, Inputs.CompileCommand.Output); - return llvm::Error::success(); + return ModuleFile{ModuleName, Inputs.CompileCommand.Output}; +} + +bool ReusablePrerequisiteModules::canReuse( +const CompilerInvocation &CI, +llvm::IntrusiveRefCntPtr VFS) const { + if (RequiredModules.empty()) +return true; + + SmallVector BMIPaths; + for (auto &MF : RequiredModules) +BMIPaths.push_back(MF->ModuleFilePath); + return IsModuleFilesUpToDate(BMIPaths, *this, VFS); } } // namespace +class ModulesBuilder::ModuleFileCache { +public: + ModuleFileCache(const GlobalCompilationDatabase &CDB) : CDB(CDB) {} + + llvm::Error + getOrBuildModuleFile(StringRef ModuleName, const ThreadsafeFS &TFS, + ProjectModules &MDB, + ReusablePrerequisiteModules &RequiredModules); + const GlobalCompilationDatabase &getCDB() const { return CDB; } + +private: + std::shared_ptr + getValidModuleFile(StringRef ModuleName, ProjectModules &MDB, + const ThreadsafeFS &TFS, + PrerequisiteModules &BuiltModuleFiles); + + /// This should only be called by getValidModuleFile. This is unlocked version + /// of getValidModuleFile. The function is extracted to avoid dead locks when + /// recursing. + std::shared_ptr + isValidModuleFileUnlocked(StringRef ModuleName, ProjectModules &MDB, +const ThreadsafeFS &TFS, +PrerequisiteModules &BuiltModuleFiles); + + const GlobalCompilationDatabase &CDB; + + llvm::StringMap> ModuleFiles; + // Mutex to guard accesses to ModuleFiles. + std::mutex ModuleFilesMutex; +}; + +std::shared_ptr +ModulesBuilder::ModuleFileCache::isValidModuleFileUnlocked( +StringRef ModuleName, ProjectModules &MDB, const ThreadsafeFS &TFS, +PrerequisiteModules &BuiltModuleFiles) { + auto Iter = ModuleFiles.find(ModuleName); + if (Iter != ModuleFiles.end()) { +if (!IsModuleFileUpToDate(Iter->second->ModuleFilePath, BuiltModuleFiles, + TFS.view(std::nullopt))) { + log("Found not-up-date module file {0} for module {1} in cache", + Iter->second->ModuleFilePath, ModuleName); + ModuleFiles.erase(Iter); + return nullptr; +} + +if (llvm::any_of( +MDB.getRequiredModules(MDB.getSourceForModuleName(ModuleName)), +[&MDB, &TFS, &BuiltModuleFiles, this](auto &&RequiredModuleName) { + return !isValidModuleFileUnlocked(RequiredModuleName, MDB, TFS, +BuiltModuleFiles); +})) { + ModuleFiles.erase(Iter); + return nullptr; +} + +return Iter->second; + } + + log("Don't find {0} in cache", ModuleName); + + return nullptr; +} + +std::shared_ptr ModulesBuilder::ModuleFileCache::getValidModuleFile( +StringRef ModuleName, ProjectModules &MDB, const ThreadsafeFS &TFS, +PrerequisiteModules &BuiltModuleFiles) { + std::lock_guard _(ModuleFilesMutex); + + return isValidModuleFileUnlocked(ModuleName, MDB, TFS, BuiltModuleFiles); +} + +llvm::Error ModulesBuilder::ModuleFileCache::getOrBuildModuleFile( kadircet wrote: i think we achieved the bit around cache just being a cache, but understanding of building a module is still split across two places. as `ModulesBuilderImpl` needs to know details of `buildModuleFile` (as it ensures to call this function only after all the dependent modules are traversed and build), and `buildModuleFile` knows the other bit around invoking a compilation and trusting it will only be called after all the dependencies are build. Can we make sure all of this logic is contained in one place instead? https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
@@ -316,36 +287,169 @@ llvm::Error buildModuleFile(llvm::StringRef ModuleName, if (Clang->getDiagnostics().hasErrorOccurred()) return llvm::createStringError("Compilation failed"); - BuiltModuleFiles.addModuleFile(ModuleName, Inputs.CompileCommand.Output); - return llvm::Error::success(); + return ModuleFile{ModuleName, Inputs.CompileCommand.Output}; +} + +bool ReusablePrerequisiteModules::canReuse( +const CompilerInvocation &CI, +llvm::IntrusiveRefCntPtr VFS) const { + if (RequiredModules.empty()) +return true; + + SmallVector BMIPaths; + for (auto &MF : RequiredModules) +BMIPaths.push_back(MF->ModuleFilePath); + return IsModuleFilesUpToDate(BMIPaths, *this, VFS); } } // namespace +class ModulesBuilder::ModuleFileCache { +public: + ModuleFileCache(const GlobalCompilationDatabase &CDB) : CDB(CDB) {} + + llvm::Error + getOrBuildModuleFile(StringRef ModuleName, const ThreadsafeFS &TFS, + ProjectModules &MDB, + ReusablePrerequisiteModules &RequiredModules); + const GlobalCompilationDatabase &getCDB() const { return CDB; } + +private: + std::shared_ptr + getValidModuleFile(StringRef ModuleName, ProjectModules &MDB, + const ThreadsafeFS &TFS, + PrerequisiteModules &BuiltModuleFiles); + + /// This should only be called by getValidModuleFile. This is unlocked version + /// of getValidModuleFile. The function is extracted to avoid dead locks when + /// recursing. + std::shared_ptr + isValidModuleFileUnlocked(StringRef ModuleName, ProjectModules &MDB, +const ThreadsafeFS &TFS, +PrerequisiteModules &BuiltModuleFiles); + + const GlobalCompilationDatabase &CDB; + + llvm::StringMap> ModuleFiles; kadircet wrote: > The cache should be an owner. Otherwise, if we close all tabs and we open a > tab again, we may have to build the same module file that could be in the > cache. Yes, and I think that's WAI. That's what clangd has today with preambles, if you close a file and re-open it, you'll pay for the warm up cost. This ensures we can keep a long editing session going on. We ensure we're clearing up the resources as people close their files. Doing it differently here will imply clangd can no longer be used in workflows that open tons of files while navigating the codebase. As nothing will clear up the dependent modules built in the meanwhile. We're still going to provide better performance here, compared to preambles. As we'll re-use any "core" modules that we keep in the cache. https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
@@ -228,54 +223,30 @@ class StandalonePrerequisiteModules : public PrerequisiteModules { return BuiltModuleNames.contains(ModuleName); } - void addModuleFile(llvm::StringRef ModuleName, - llvm::StringRef ModuleFilePath) { -RequiredModules.emplace_back(ModuleName, ModuleFilePath); -BuiltModuleNames.insert(ModuleName); + void addModuleFile(std::shared_ptr BMI) { +BuiltModuleNames.insert(BMI->ModuleName); +RequiredModules.emplace_back(std::move(BMI)); } private: - llvm::SmallVector RequiredModules; + mutable llvm::SmallVector, 8> RequiredModules; kadircet wrote: > I add the data members of ModuleFile to be private and we can only access > them by the getters. I think this has the same effect. i don't think it has the same affect. It makes sure code today is adhering to those standards because you and me have context and very careful about the interaction. Tomorrow someone might end up mutating ModuleFiles because the interfaces in builder didn't enforce that and they didn't pay as much attention to subtle shared ownership model. https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
@@ -316,36 +294,205 @@ llvm::Error buildModuleFile(llvm::StringRef ModuleName, if (Clang->getDiagnostics().hasErrorOccurred()) return llvm::createStringError("Compilation failed"); - BuiltModuleFiles.addModuleFile(ModuleName, Inputs.CompileCommand.Output); - return llvm::Error::success(); + return ModuleFile{ModuleName, Inputs.CompileCommand.Output}; +} + +bool ReusablePrerequisiteModules::canReuse( +const CompilerInvocation &CI, +llvm::IntrusiveRefCntPtr VFS) const { + if (RequiredModules.empty()) +return true; + + SmallVector BMIPaths; + for (auto &MF : RequiredModules) +BMIPaths.push_back(MF->getModuleFilePath()); + return IsModuleFilesUpToDate(BMIPaths, *this, VFS); +} + +class ModuleFileCache { +public: + ModuleFileCache(const GlobalCompilationDatabase &CDB) : CDB(CDB) {} + + llvm::Error + getOrBuildModuleFile(StringRef ModuleName, const ThreadsafeFS &TFS, + ProjectModules &MDB, + ReusablePrerequisiteModules &RequiredModules); + const GlobalCompilationDatabase &getCDB() const { return CDB; } + + std::shared_ptr + getValidModuleFile(StringRef ModuleName, ProjectModules &MDB, + const ThreadsafeFS &TFS, + PrerequisiteModules &BuiltModuleFiles); + + void add(StringRef ModuleName, std::shared_ptr ModuleFile) { +std::lock_guard _(ModuleFilesMutex); + +ModuleFiles.insert_or_assign(ModuleName, ModuleFile); + } + +private: + const GlobalCompilationDatabase &CDB; + + llvm::StringMap> ModuleFiles; + // Mutex to guard accesses to ModuleFiles. + std::mutex ModuleFilesMutex; +}; + +/// Collect the directly and indirectly required module names for \param +/// ModuleName. The \param ModuleName is guaranteed to be the first element in +/// \param ModuleNames. +void getAllRequiredModules(ProjectModules &MDB, StringRef ModuleName, + llvm::SmallVector &ModuleNames) { + std::queue Worklist; + llvm::StringSet<> ModuleNamesSet; + Worklist.push(ModuleName); + + while (!Worklist.empty()) { +StringRef CurrentModule = Worklist.front(); +Worklist.pop(); + +if (!ModuleNamesSet.insert(CurrentModule).second) + continue; + +ModuleNames.push_back(CurrentModule); + +for (StringRef RequiredModuleName : + MDB.getRequiredModules(MDB.getSourceForModuleName(CurrentModule))) + if (!ModuleNamesSet.contains(RequiredModuleName)) +Worklist.push(RequiredModuleName); + } +} + +std::shared_ptr +ModuleFileCache::getValidModuleFile(StringRef ModuleName, ProjectModules &MDB, +const ThreadsafeFS &TFS, +PrerequisiteModules &BuiltModuleFiles) { + { +std::lock_guard _(ModuleFilesMutex); kadircet wrote: call it `Lock` (same below) https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
@@ -316,36 +294,205 @@ llvm::Error buildModuleFile(llvm::StringRef ModuleName, if (Clang->getDiagnostics().hasErrorOccurred()) return llvm::createStringError("Compilation failed"); - BuiltModuleFiles.addModuleFile(ModuleName, Inputs.CompileCommand.Output); - return llvm::Error::success(); + return ModuleFile{ModuleName, Inputs.CompileCommand.Output}; +} + +bool ReusablePrerequisiteModules::canReuse( +const CompilerInvocation &CI, +llvm::IntrusiveRefCntPtr VFS) const { + if (RequiredModules.empty()) +return true; + + SmallVector BMIPaths; + for (auto &MF : RequiredModules) +BMIPaths.push_back(MF->getModuleFilePath()); + return IsModuleFilesUpToDate(BMIPaths, *this, VFS); +} + +class ModuleFileCache { +public: + ModuleFileCache(const GlobalCompilationDatabase &CDB) : CDB(CDB) {} + + llvm::Error + getOrBuildModuleFile(StringRef ModuleName, const ThreadsafeFS &TFS, + ProjectModules &MDB, + ReusablePrerequisiteModules &RequiredModules); + const GlobalCompilationDatabase &getCDB() const { return CDB; } + + std::shared_ptr + getValidModuleFile(StringRef ModuleName, ProjectModules &MDB, + const ThreadsafeFS &TFS, + PrerequisiteModules &BuiltModuleFiles); + + void add(StringRef ModuleName, std::shared_ptr ModuleFile) { +std::lock_guard _(ModuleFilesMutex); + +ModuleFiles.insert_or_assign(ModuleName, ModuleFile); + } + +private: + const GlobalCompilationDatabase &CDB; + + llvm::StringMap> ModuleFiles; + // Mutex to guard accesses to ModuleFiles. + std::mutex ModuleFilesMutex; +}; + +/// Collect the directly and indirectly required module names for \param +/// ModuleName. The \param ModuleName is guaranteed to be the first element in +/// \param ModuleNames. +void getAllRequiredModules(ProjectModules &MDB, StringRef ModuleName, + llvm::SmallVector &ModuleNames) { + std::queue Worklist; + llvm::StringSet<> ModuleNamesSet; + Worklist.push(ModuleName); + + while (!Worklist.empty()) { +StringRef CurrentModule = Worklist.front(); +Worklist.pop(); + +if (!ModuleNamesSet.insert(CurrentModule).second) + continue; + +ModuleNames.push_back(CurrentModule); + +for (StringRef RequiredModuleName : + MDB.getRequiredModules(MDB.getSourceForModuleName(CurrentModule))) + if (!ModuleNamesSet.contains(RequiredModuleName)) +Worklist.push(RequiredModuleName); + } +} + +std::shared_ptr +ModuleFileCache::getValidModuleFile(StringRef ModuleName, ProjectModules &MDB, +const ThreadsafeFS &TFS, +PrerequisiteModules &BuiltModuleFiles) { + { +std::lock_guard _(ModuleFilesMutex); + +if (ModuleFiles.find(ModuleName) == ModuleFiles.end()) + return nullptr; + } + + llvm::SmallVector ModuleNames; + getAllRequiredModules(MDB, ModuleName, ModuleNames); + + llvm::SmallVector> RequiredModuleFiles; + + { +std::lock_guard _(ModuleFilesMutex); + +for (StringRef ModuleName : ModuleNames) { + auto Iter = ModuleFiles.find(ModuleName); + if (Iter == ModuleFiles.end()) +return nullptr; + + RequiredModuleFiles.push_back(Iter->second); +} + } + + if (RequiredModuleFiles.empty()) +return nullptr; + + if (llvm::all_of(RequiredModuleFiles, [&](auto MF) { kadircet wrote: use `any_of` and invert the `IsModuleFileUpToDate` check to bail out early here. https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
@@ -316,36 +294,205 @@ llvm::Error buildModuleFile(llvm::StringRef ModuleName, if (Clang->getDiagnostics().hasErrorOccurred()) return llvm::createStringError("Compilation failed"); - BuiltModuleFiles.addModuleFile(ModuleName, Inputs.CompileCommand.Output); - return llvm::Error::success(); + return ModuleFile{ModuleName, Inputs.CompileCommand.Output}; +} + +bool ReusablePrerequisiteModules::canReuse( +const CompilerInvocation &CI, +llvm::IntrusiveRefCntPtr VFS) const { + if (RequiredModules.empty()) +return true; + + SmallVector BMIPaths; + for (auto &MF : RequiredModules) +BMIPaths.push_back(MF->getModuleFilePath()); + return IsModuleFilesUpToDate(BMIPaths, *this, VFS); +} + +class ModuleFileCache { +public: + ModuleFileCache(const GlobalCompilationDatabase &CDB) : CDB(CDB) {} + + llvm::Error + getOrBuildModuleFile(StringRef ModuleName, const ThreadsafeFS &TFS, + ProjectModules &MDB, + ReusablePrerequisiteModules &RequiredModules); + const GlobalCompilationDatabase &getCDB() const { return CDB; } + + std::shared_ptr + getValidModuleFile(StringRef ModuleName, ProjectModules &MDB, + const ThreadsafeFS &TFS, + PrerequisiteModules &BuiltModuleFiles); + + void add(StringRef ModuleName, std::shared_ptr ModuleFile) { +std::lock_guard _(ModuleFilesMutex); + +ModuleFiles.insert_or_assign(ModuleName, ModuleFile); + } + +private: + const GlobalCompilationDatabase &CDB; + + llvm::StringMap> ModuleFiles; + // Mutex to guard accesses to ModuleFiles. + std::mutex ModuleFilesMutex; +}; + +/// Collect the directly and indirectly required module names for \param +/// ModuleName. The \param ModuleName is guaranteed to be the first element in +/// \param ModuleNames. +void getAllRequiredModules(ProjectModules &MDB, StringRef ModuleName, + llvm::SmallVector &ModuleNames) { + std::queue Worklist; + llvm::StringSet<> ModuleNamesSet; + Worklist.push(ModuleName); + + while (!Worklist.empty()) { +StringRef CurrentModule = Worklist.front(); +Worklist.pop(); + +if (!ModuleNamesSet.insert(CurrentModule).second) + continue; + +ModuleNames.push_back(CurrentModule); + +for (StringRef RequiredModuleName : + MDB.getRequiredModules(MDB.getSourceForModuleName(CurrentModule))) + if (!ModuleNamesSet.contains(RequiredModuleName)) +Worklist.push(RequiredModuleName); + } +} + +std::shared_ptr +ModuleFileCache::getValidModuleFile(StringRef ModuleName, ProjectModules &MDB, +const ThreadsafeFS &TFS, +PrerequisiteModules &BuiltModuleFiles) { + { +std::lock_guard _(ModuleFilesMutex); + +if (ModuleFiles.find(ModuleName) == ModuleFiles.end()) + return nullptr; + } + + llvm::SmallVector ModuleNames; + getAllRequiredModules(MDB, ModuleName, ModuleNames); + + llvm::SmallVector> RequiredModuleFiles; + + { +std::lock_guard _(ModuleFilesMutex); + +for (StringRef ModuleName : ModuleNames) { + auto Iter = ModuleFiles.find(ModuleName); + if (Iter == ModuleFiles.end()) +return nullptr; + + RequiredModuleFiles.push_back(Iter->second); +} + } + + if (RequiredModuleFiles.empty()) +return nullptr; kadircet wrote: we already returned nullptr if we're missing **any** module-files in the loop above https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
@@ -316,36 +294,205 @@ llvm::Error buildModuleFile(llvm::StringRef ModuleName, if (Clang->getDiagnostics().hasErrorOccurred()) return llvm::createStringError("Compilation failed"); - BuiltModuleFiles.addModuleFile(ModuleName, Inputs.CompileCommand.Output); - return llvm::Error::success(); + return ModuleFile{ModuleName, Inputs.CompileCommand.Output}; +} + +bool ReusablePrerequisiteModules::canReuse( +const CompilerInvocation &CI, +llvm::IntrusiveRefCntPtr VFS) const { + if (RequiredModules.empty()) +return true; + + SmallVector BMIPaths; + for (auto &MF : RequiredModules) +BMIPaths.push_back(MF->getModuleFilePath()); + return IsModuleFilesUpToDate(BMIPaths, *this, VFS); +} + +class ModuleFileCache { +public: + ModuleFileCache(const GlobalCompilationDatabase &CDB) : CDB(CDB) {} + + llvm::Error + getOrBuildModuleFile(StringRef ModuleName, const ThreadsafeFS &TFS, + ProjectModules &MDB, + ReusablePrerequisiteModules &RequiredModules); + const GlobalCompilationDatabase &getCDB() const { return CDB; } + + std::shared_ptr + getValidModuleFile(StringRef ModuleName, ProjectModules &MDB, + const ThreadsafeFS &TFS, + PrerequisiteModules &BuiltModuleFiles); + + void add(StringRef ModuleName, std::shared_ptr ModuleFile) { +std::lock_guard _(ModuleFilesMutex); + +ModuleFiles.insert_or_assign(ModuleName, ModuleFile); + } + +private: + const GlobalCompilationDatabase &CDB; + + llvm::StringMap> ModuleFiles; + // Mutex to guard accesses to ModuleFiles. + std::mutex ModuleFilesMutex; +}; + +/// Collect the directly and indirectly required module names for \param +/// ModuleName. The \param ModuleName is guaranteed to be the first element in +/// \param ModuleNames. +void getAllRequiredModules(ProjectModules &MDB, StringRef ModuleName, + llvm::SmallVector &ModuleNames) { + std::queue Worklist; + llvm::StringSet<> ModuleNamesSet; + Worklist.push(ModuleName); + + while (!Worklist.empty()) { +StringRef CurrentModule = Worklist.front(); +Worklist.pop(); + +if (!ModuleNamesSet.insert(CurrentModule).second) + continue; + +ModuleNames.push_back(CurrentModule); + +for (StringRef RequiredModuleName : + MDB.getRequiredModules(MDB.getSourceForModuleName(CurrentModule))) + if (!ModuleNamesSet.contains(RequiredModuleName)) kadircet wrote: let's perform the `insert` here and check if it succeeded to prevent extra loops https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
@@ -316,36 +294,205 @@ llvm::Error buildModuleFile(llvm::StringRef ModuleName, if (Clang->getDiagnostics().hasErrorOccurred()) return llvm::createStringError("Compilation failed"); - BuiltModuleFiles.addModuleFile(ModuleName, Inputs.CompileCommand.Output); - return llvm::Error::success(); + return ModuleFile{ModuleName, Inputs.CompileCommand.Output}; +} + +bool ReusablePrerequisiteModules::canReuse( +const CompilerInvocation &CI, +llvm::IntrusiveRefCntPtr VFS) const { + if (RequiredModules.empty()) +return true; + + SmallVector BMIPaths; + for (auto &MF : RequiredModules) +BMIPaths.push_back(MF->getModuleFilePath()); + return IsModuleFilesUpToDate(BMIPaths, *this, VFS); +} + +class ModuleFileCache { +public: + ModuleFileCache(const GlobalCompilationDatabase &CDB) : CDB(CDB) {} + + llvm::Error + getOrBuildModuleFile(StringRef ModuleName, const ThreadsafeFS &TFS, + ProjectModules &MDB, + ReusablePrerequisiteModules &RequiredModules); + const GlobalCompilationDatabase &getCDB() const { return CDB; } + + std::shared_ptr + getValidModuleFile(StringRef ModuleName, ProjectModules &MDB, + const ThreadsafeFS &TFS, + PrerequisiteModules &BuiltModuleFiles); + + void add(StringRef ModuleName, std::shared_ptr ModuleFile) { +std::lock_guard _(ModuleFilesMutex); + +ModuleFiles.insert_or_assign(ModuleName, ModuleFile); + } + +private: + const GlobalCompilationDatabase &CDB; + + llvm::StringMap> ModuleFiles; + // Mutex to guard accesses to ModuleFiles. + std::mutex ModuleFilesMutex; +}; + +/// Collect the directly and indirectly required module names for \param +/// ModuleName. The \param ModuleName is guaranteed to be the first element in +/// \param ModuleNames. +void getAllRequiredModules(ProjectModules &MDB, StringRef ModuleName, + llvm::SmallVector &ModuleNames) { + std::queue Worklist; + llvm::StringSet<> ModuleNamesSet; + Worklist.push(ModuleName); + + while (!Worklist.empty()) { +StringRef CurrentModule = Worklist.front(); +Worklist.pop(); + +if (!ModuleNamesSet.insert(CurrentModule).second) + continue; + +ModuleNames.push_back(CurrentModule); + +for (StringRef RequiredModuleName : + MDB.getRequiredModules(MDB.getSourceForModuleName(CurrentModule))) + if (!ModuleNamesSet.contains(RequiredModuleName)) +Worklist.push(RequiredModuleName); + } +} + +std::shared_ptr +ModuleFileCache::getValidModuleFile(StringRef ModuleName, ProjectModules &MDB, +const ThreadsafeFS &TFS, +PrerequisiteModules &BuiltModuleFiles) { + { +std::lock_guard _(ModuleFilesMutex); + +if (ModuleFiles.find(ModuleName) == ModuleFiles.end()) kadircet wrote: prefer `.count` https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
@@ -316,36 +294,205 @@ llvm::Error buildModuleFile(llvm::StringRef ModuleName, if (Clang->getDiagnostics().hasErrorOccurred()) return llvm::createStringError("Compilation failed"); - BuiltModuleFiles.addModuleFile(ModuleName, Inputs.CompileCommand.Output); - return llvm::Error::success(); + return ModuleFile{ModuleName, Inputs.CompileCommand.Output}; +} + +bool ReusablePrerequisiteModules::canReuse( +const CompilerInvocation &CI, +llvm::IntrusiveRefCntPtr VFS) const { + if (RequiredModules.empty()) +return true; + + SmallVector BMIPaths; + for (auto &MF : RequiredModules) +BMIPaths.push_back(MF->getModuleFilePath()); + return IsModuleFilesUpToDate(BMIPaths, *this, VFS); +} + +class ModuleFileCache { +public: + ModuleFileCache(const GlobalCompilationDatabase &CDB) : CDB(CDB) {} + + llvm::Error + getOrBuildModuleFile(StringRef ModuleName, const ThreadsafeFS &TFS, + ProjectModules &MDB, + ReusablePrerequisiteModules &RequiredModules); + const GlobalCompilationDatabase &getCDB() const { return CDB; } + + std::shared_ptr + getValidModuleFile(StringRef ModuleName, ProjectModules &MDB, + const ThreadsafeFS &TFS, + PrerequisiteModules &BuiltModuleFiles); + + void add(StringRef ModuleName, std::shared_ptr ModuleFile) { +std::lock_guard _(ModuleFilesMutex); + +ModuleFiles.insert_or_assign(ModuleName, ModuleFile); + } + +private: + const GlobalCompilationDatabase &CDB; + + llvm::StringMap> ModuleFiles; + // Mutex to guard accesses to ModuleFiles. + std::mutex ModuleFilesMutex; +}; + +/// Collect the directly and indirectly required module names for \param +/// ModuleName. The \param ModuleName is guaranteed to be the first element in +/// \param ModuleNames. +void getAllRequiredModules(ProjectModules &MDB, StringRef ModuleName, kadircet wrote: can you just return `std::vector` instead of taking an out parameter here? https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
https://github.com/kadircet requested changes to this pull request. https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
ChuanqiXu9 wrote: > thanks I think this LG in terms of module-builder interfaces, but I think we > can make some more changes to implementation to ensure it's easier to > maintain going forward. > > speaking of maintenance, @HighCommander4 is definitely doing more of that > than me recently. so i'd like to make sure he's also ok with this new > complexity. i also would like to highlight @ChuanqiXu9 is definitely a > responsible member of community, hence i believe he'll also be around if we > have bug reports about this bits. Thanks : ) https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
@@ -482,6 +482,42 @@ void func() { EXPECT_EQ(Result.signatures[0].parameters[0].labelString, "int a"); } +TEST_F(PrerequisiteModulesTests, ReusablePrerequisiteModulesTest) { ChuanqiXu9 wrote: Done by appending a new test. https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
@@ -316,36 +287,169 @@ llvm::Error buildModuleFile(llvm::StringRef ModuleName, if (Clang->getDiagnostics().hasErrorOccurred()) return llvm::createStringError("Compilation failed"); - BuiltModuleFiles.addModuleFile(ModuleName, Inputs.CompileCommand.Output); - return llvm::Error::success(); + return ModuleFile{ModuleName, Inputs.CompileCommand.Output}; +} + +bool ReusablePrerequisiteModules::canReuse( +const CompilerInvocation &CI, +llvm::IntrusiveRefCntPtr VFS) const { + if (RequiredModules.empty()) +return true; + + SmallVector BMIPaths; + for (auto &MF : RequiredModules) +BMIPaths.push_back(MF->ModuleFilePath); + return IsModuleFilesUpToDate(BMIPaths, *this, VFS); } } // namespace +class ModulesBuilder::ModuleFileCache { +public: + ModuleFileCache(const GlobalCompilationDatabase &CDB) : CDB(CDB) {} + + llvm::Error + getOrBuildModuleFile(StringRef ModuleName, const ThreadsafeFS &TFS, + ProjectModules &MDB, + ReusablePrerequisiteModules &RequiredModules); + const GlobalCompilationDatabase &getCDB() const { return CDB; } + +private: + std::shared_ptr + getValidModuleFile(StringRef ModuleName, ProjectModules &MDB, + const ThreadsafeFS &TFS, + PrerequisiteModules &BuiltModuleFiles); + + /// This should only be called by getValidModuleFile. This is unlocked version + /// of getValidModuleFile. The function is extracted to avoid dead locks when + /// recursing. + std::shared_ptr + isValidModuleFileUnlocked(StringRef ModuleName, ProjectModules &MDB, +const ThreadsafeFS &TFS, +PrerequisiteModules &BuiltModuleFiles); + + const GlobalCompilationDatabase &CDB; + + llvm::StringMap> ModuleFiles; + // Mutex to guard accesses to ModuleFiles. + std::mutex ModuleFilesMutex; +}; + +std::shared_ptr +ModulesBuilder::ModuleFileCache::isValidModuleFileUnlocked( +StringRef ModuleName, ProjectModules &MDB, const ThreadsafeFS &TFS, +PrerequisiteModules &BuiltModuleFiles) { + auto Iter = ModuleFiles.find(ModuleName); + if (Iter != ModuleFiles.end()) { +if (!IsModuleFileUpToDate(Iter->second->ModuleFilePath, BuiltModuleFiles, + TFS.view(std::nullopt))) { + log("Found not-up-date module file {0} for module {1} in cache", + Iter->second->ModuleFilePath, ModuleName); + ModuleFiles.erase(Iter); + return nullptr; +} + +if (llvm::any_of( +MDB.getRequiredModules(MDB.getSourceForModuleName(ModuleName)), +[&MDB, &TFS, &BuiltModuleFiles, this](auto &&RequiredModuleName) { + return !isValidModuleFileUnlocked(RequiredModuleName, MDB, TFS, +BuiltModuleFiles); +})) { + ModuleFiles.erase(Iter); + return nullptr; +} + +return Iter->second; + } + + log("Don't find {0} in cache", ModuleName); + + return nullptr; +} + +std::shared_ptr ModulesBuilder::ModuleFileCache::getValidModuleFile( +StringRef ModuleName, ProjectModules &MDB, const ThreadsafeFS &TFS, +PrerequisiteModules &BuiltModuleFiles) { + std::lock_guard _(ModuleFilesMutex); + + return isValidModuleFileUnlocked(ModuleName, MDB, TFS, BuiltModuleFiles); +} + +llvm::Error ModulesBuilder::ModuleFileCache::getOrBuildModuleFile( ChuanqiXu9 wrote: I can understand your point as you wish the cache to be a cache only. And I feel good to leave the free function `buildModuleFile` as simple as possible so that it doesn't have to care about the cache. To address both opinions, I tried to add `ModulesBuilderImpl` class to take the place of the previous ModuleFilesCache and the current ModuleFileCache is a cache only. I feel this is the neater solution. https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
@@ -228,54 +223,30 @@ class StandalonePrerequisiteModules : public PrerequisiteModules { return BuiltModuleNames.contains(ModuleName); } - void addModuleFile(llvm::StringRef ModuleName, - llvm::StringRef ModuleFilePath) { -RequiredModules.emplace_back(ModuleName, ModuleFilePath); -BuiltModuleNames.insert(ModuleName); + void addModuleFile(std::shared_ptr BMI) { +BuiltModuleNames.insert(BMI->ModuleName); +RequiredModules.emplace_back(std::move(BMI)); } private: - llvm::SmallVector RequiredModules; + mutable llvm::SmallVector, 8> RequiredModules; // A helper class to speedup the query if a module is built. llvm::StringSet<> BuiltModuleNames; }; -// Build a module file for module with `ModuleName`. The information of built -// module file are stored in \param BuiltModuleFiles. -llvm::Error buildModuleFile(llvm::StringRef ModuleName, -const GlobalCompilationDatabase &CDB, -const ThreadsafeFS &TFS, ProjectModules &MDB, -PathRef ModuleFilesPrefix, -StandalonePrerequisiteModules &BuiltModuleFiles) { - if (BuiltModuleFiles.isModuleUnitBuilt(ModuleName)) -return llvm::Error::success(); - - PathRef ModuleUnitFileName = MDB.getSourceForModuleName(ModuleName); - // It is possible that we're meeting third party modules (modules whose - // source are not in the project. e.g, the std module may be a third-party - // module for most projects) or something wrong with the implementation of - // ProjectModules. - // FIXME: How should we treat third party modules here? If we want to ignore - // third party modules, we should return true instead of false here. - // Currently we simply bail out. - if (ModuleUnitFileName.empty()) -return llvm::createStringError("Failed to get the primary source"); - +/// Build a module file for module with `ModuleName`. The information of built +/// module file are stored in \param BuiltModuleFiles. +llvm::Expected +buildModuleFile(llvm::StringRef ModuleName, PathRef ModuleUnitFileName, +const GlobalCompilationDatabase &CDB, const ThreadsafeFS &TFS, +PathRef ModuleFilesPrefix, +const ReusablePrerequisiteModules &BuiltModuleFiles) { // Try cheap operation earlier to boil-out cheaply if there are problems. auto Cmd = CDB.getCompileCommand(ModuleUnitFileName); if (!Cmd) return llvm::createStringError( llvm::formatv("No compile command for {0}", ModuleUnitFileName)); - for (auto &RequiredModuleName : MDB.getRequiredModules(ModuleUnitFileName)) { ChuanqiXu9 wrote: See the following comment. https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
@@ -228,54 +223,30 @@ class StandalonePrerequisiteModules : public PrerequisiteModules { return BuiltModuleNames.contains(ModuleName); } - void addModuleFile(llvm::StringRef ModuleName, - llvm::StringRef ModuleFilePath) { -RequiredModules.emplace_back(ModuleName, ModuleFilePath); -BuiltModuleNames.insert(ModuleName); + void addModuleFile(std::shared_ptr BMI) { +BuiltModuleNames.insert(BMI->ModuleName); +RequiredModules.emplace_back(std::move(BMI)); } private: - llvm::SmallVector RequiredModules; + mutable llvm::SmallVector, 8> RequiredModules; ChuanqiXu9 wrote: It is an oversight to mark it as mutable. I add the data members of `ModuleFile ` to be private and we can only access them by the getters. I think this has the same effect. https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
@@ -316,36 +287,169 @@ llvm::Error buildModuleFile(llvm::StringRef ModuleName, if (Clang->getDiagnostics().hasErrorOccurred()) return llvm::createStringError("Compilation failed"); - BuiltModuleFiles.addModuleFile(ModuleName, Inputs.CompileCommand.Output); - return llvm::Error::success(); + return ModuleFile{ModuleName, Inputs.CompileCommand.Output}; +} + +bool ReusablePrerequisiteModules::canReuse( +const CompilerInvocation &CI, +llvm::IntrusiveRefCntPtr VFS) const { + if (RequiredModules.empty()) +return true; + + SmallVector BMIPaths; + for (auto &MF : RequiredModules) +BMIPaths.push_back(MF->ModuleFilePath); + return IsModuleFilesUpToDate(BMIPaths, *this, VFS); } } // namespace +class ModulesBuilder::ModuleFileCache { +public: + ModuleFileCache(const GlobalCompilationDatabase &CDB) : CDB(CDB) {} + + llvm::Error + getOrBuildModuleFile(StringRef ModuleName, const ThreadsafeFS &TFS, + ProjectModules &MDB, + ReusablePrerequisiteModules &RequiredModules); + const GlobalCompilationDatabase &getCDB() const { return CDB; } + +private: + std::shared_ptr + getValidModuleFile(StringRef ModuleName, ProjectModules &MDB, + const ThreadsafeFS &TFS, + PrerequisiteModules &BuiltModuleFiles); + + /// This should only be called by getValidModuleFile. This is unlocked version + /// of getValidModuleFile. The function is extracted to avoid dead locks when + /// recursing. + std::shared_ptr + isValidModuleFileUnlocked(StringRef ModuleName, ProjectModules &MDB, +const ThreadsafeFS &TFS, +PrerequisiteModules &BuiltModuleFiles); + + const GlobalCompilationDatabase &CDB; + + llvm::StringMap> ModuleFiles; ChuanqiXu9 wrote: I think `std::shared_ptr` is correct here. The cache should be an owner. Otherwise, if we close all tabs and we open a tab again, we may have to build the same module file that could be in the cache. https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
@@ -316,36 +287,169 @@ llvm::Error buildModuleFile(llvm::StringRef ModuleName, if (Clang->getDiagnostics().hasErrorOccurred()) return llvm::createStringError("Compilation failed"); - BuiltModuleFiles.addModuleFile(ModuleName, Inputs.CompileCommand.Output); - return llvm::Error::success(); + return ModuleFile{ModuleName, Inputs.CompileCommand.Output}; +} + +bool ReusablePrerequisiteModules::canReuse( +const CompilerInvocation &CI, +llvm::IntrusiveRefCntPtr VFS) const { + if (RequiredModules.empty()) +return true; + + SmallVector BMIPaths; + for (auto &MF : RequiredModules) +BMIPaths.push_back(MF->ModuleFilePath); + return IsModuleFilesUpToDate(BMIPaths, *this, VFS); } } // namespace +class ModulesBuilder::ModuleFileCache { +public: + ModuleFileCache(const GlobalCompilationDatabase &CDB) : CDB(CDB) {} + + llvm::Error + getOrBuildModuleFile(StringRef ModuleName, const ThreadsafeFS &TFS, + ProjectModules &MDB, + ReusablePrerequisiteModules &RequiredModules); + const GlobalCompilationDatabase &getCDB() const { return CDB; } + +private: + std::shared_ptr + getValidModuleFile(StringRef ModuleName, ProjectModules &MDB, + const ThreadsafeFS &TFS, + PrerequisiteModules &BuiltModuleFiles); + + /// This should only be called by getValidModuleFile. This is unlocked version + /// of getValidModuleFile. The function is extracted to avoid dead locks when + /// recursing. + std::shared_ptr + isValidModuleFileUnlocked(StringRef ModuleName, ProjectModules &MDB, +const ThreadsafeFS &TFS, +PrerequisiteModules &BuiltModuleFiles); + + const GlobalCompilationDatabase &CDB; + + llvm::StringMap> ModuleFiles; + // Mutex to guard accesses to ModuleFiles. + std::mutex ModuleFilesMutex; +}; + +std::shared_ptr +ModulesBuilder::ModuleFileCache::isValidModuleFileUnlocked( ChuanqiXu9 wrote: Done. I was uncomfortable slightly since we lost the chance to erase invalided entries in the cache now. But the things cache are never guaranteed to be valid. https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
https://github.com/ChuanqiXu9 updated https://github.com/llvm/llvm-project/pull/106683 >From 15aa6af1f5d22e4b837e8e2fd49469310ffbe7f1 Mon Sep 17 00:00:00 2001 From: Chuanqi Xu Date: Fri, 30 Aug 2024 15:11:07 +0800 Subject: [PATCH 1/3] [clangd] [Modules] Support Reusable Modules Builder --- clang-tools-extra/clangd/ModulesBuilder.cpp | 375 ++ clang-tools-extra/clangd/ModulesBuilder.h | 8 +- .../unittests/PrerequisiteModulesTest.cpp | 36 ++ 3 files changed, 344 insertions(+), 75 deletions(-) diff --git a/clang-tools-extra/clangd/ModulesBuilder.cpp b/clang-tools-extra/clangd/ModulesBuilder.cpp index 97f67ddf5495a2..c4fb3d4010d370 100644 --- a/clang-tools-extra/clangd/ModulesBuilder.cpp +++ b/clang-tools-extra/clangd/ModulesBuilder.cpp @@ -13,6 +13,7 @@ #include "clang/Frontend/FrontendActions.h" #include "clang/Serialization/ASTReader.h" #include "clang/Serialization/InMemoryModuleCache.h" +#include "llvm/ADT/ScopeExit.h" namespace clang { namespace clangd { @@ -192,33 +193,27 @@ bool IsModuleFilesUpToDate( }); } -// StandalonePrerequisiteModules - stands for PrerequisiteModules for which all +// ReusablePrerequisiteModules - stands for PrerequisiteModules for which all // the required modules are built successfully. All the module files -// are owned by the StandalonePrerequisiteModules class. -// -// Any of the built module files won't be shared with other instances of the -// class. So that we can avoid worrying thread safety. -// -// We don't need to worry about duplicated module names here since the standard -// guarantees the module names should be unique to a program. -class StandalonePrerequisiteModules : public PrerequisiteModules { +// are owned by the modules builder. +class ReusablePrerequisiteModules : public PrerequisiteModules { public: - StandalonePrerequisiteModules() = default; + ReusablePrerequisiteModules() = default; - StandalonePrerequisiteModules(const StandalonePrerequisiteModules &) = delete; - StandalonePrerequisiteModules - operator=(const StandalonePrerequisiteModules &) = delete; - StandalonePrerequisiteModules(StandalonePrerequisiteModules &&) = delete; - StandalonePrerequisiteModules - operator=(StandalonePrerequisiteModules &&) = delete; + ReusablePrerequisiteModules(const ReusablePrerequisiteModules &) = delete; + ReusablePrerequisiteModules + operator=(const ReusablePrerequisiteModules &) = delete; + ReusablePrerequisiteModules(ReusablePrerequisiteModules &&) = delete; + ReusablePrerequisiteModules + operator=(ReusablePrerequisiteModules &&) = delete; - ~StandalonePrerequisiteModules() override = default; + ~ReusablePrerequisiteModules() override = default; void adjustHeaderSearchOptions(HeaderSearchOptions &Options) const override { // Appending all built module files. for (auto &RequiredModule : RequiredModules) Options.PrebuiltModuleFiles.insert_or_assign( - RequiredModule.ModuleName, RequiredModule.ModuleFilePath); + RequiredModule->ModuleName, RequiredModule->ModuleFilePath); } bool canReuse(const CompilerInvocation &CI, @@ -228,54 +223,30 @@ class StandalonePrerequisiteModules : public PrerequisiteModules { return BuiltModuleNames.contains(ModuleName); } - void addModuleFile(llvm::StringRef ModuleName, - llvm::StringRef ModuleFilePath) { -RequiredModules.emplace_back(ModuleName, ModuleFilePath); -BuiltModuleNames.insert(ModuleName); + void addModuleFile(std::shared_ptr BMI) { +BuiltModuleNames.insert(BMI->ModuleName); +RequiredModules.emplace_back(std::move(BMI)); } private: - llvm::SmallVector RequiredModules; + mutable llvm::SmallVector, 8> RequiredModules; // A helper class to speedup the query if a module is built. llvm::StringSet<> BuiltModuleNames; }; -// Build a module file for module with `ModuleName`. The information of built -// module file are stored in \param BuiltModuleFiles. -llvm::Error buildModuleFile(llvm::StringRef ModuleName, -const GlobalCompilationDatabase &CDB, -const ThreadsafeFS &TFS, ProjectModules &MDB, -PathRef ModuleFilesPrefix, -StandalonePrerequisiteModules &BuiltModuleFiles) { - if (BuiltModuleFiles.isModuleUnitBuilt(ModuleName)) -return llvm::Error::success(); - - PathRef ModuleUnitFileName = MDB.getSourceForModuleName(ModuleName); - // It is possible that we're meeting third party modules (modules whose - // source are not in the project. e.g, the std module may be a third-party - // module for most projects) or something wrong with the implementation of - // ProjectModules. - // FIXME: How should we treat third party modules here? If we want to ignore - // third party modules, we should return true instead of false here. - // Currently we simply bail out. - if (ModuleUnitFileName.empty()) -return llvm::createStringErro
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
@@ -228,54 +223,30 @@ class StandalonePrerequisiteModules : public PrerequisiteModules { return BuiltModuleNames.contains(ModuleName); } - void addModuleFile(llvm::StringRef ModuleName, - llvm::StringRef ModuleFilePath) { -RequiredModules.emplace_back(ModuleName, ModuleFilePath); -BuiltModuleNames.insert(ModuleName); + void addModuleFile(std::shared_ptr BMI) { +BuiltModuleNames.insert(BMI->ModuleName); +RequiredModules.emplace_back(std::move(BMI)); } private: - llvm::SmallVector RequiredModules; + mutable llvm::SmallVector, 8> RequiredModules; // A helper class to speedup the query if a module is built. llvm::StringSet<> BuiltModuleNames; }; -// Build a module file for module with `ModuleName`. The information of built -// module file are stored in \param BuiltModuleFiles. -llvm::Error buildModuleFile(llvm::StringRef ModuleName, -const GlobalCompilationDatabase &CDB, -const ThreadsafeFS &TFS, ProjectModules &MDB, -PathRef ModuleFilesPrefix, -StandalonePrerequisiteModules &BuiltModuleFiles) { - if (BuiltModuleFiles.isModuleUnitBuilt(ModuleName)) -return llvm::Error::success(); - - PathRef ModuleUnitFileName = MDB.getSourceForModuleName(ModuleName); - // It is possible that we're meeting third party modules (modules whose - // source are not in the project. e.g, the std module may be a third-party - // module for most projects) or something wrong with the implementation of - // ProjectModules. - // FIXME: How should we treat third party modules here? If we want to ignore - // third party modules, we should return true instead of false here. - // Currently we simply bail out. - if (ModuleUnitFileName.empty()) -return llvm::createStringError("Failed to get the primary source"); - +/// Build a module file for module with `ModuleName`. The information of built +/// module file are stored in \param BuiltModuleFiles. +llvm::Expected +buildModuleFile(llvm::StringRef ModuleName, PathRef ModuleUnitFileName, +const GlobalCompilationDatabase &CDB, const ThreadsafeFS &TFS, +PathRef ModuleFilesPrefix, +const ReusablePrerequisiteModules &BuiltModuleFiles) { // Try cheap operation earlier to boil-out cheaply if there are problems. auto Cmd = CDB.getCompileCommand(ModuleUnitFileName); if (!Cmd) return llvm::createStringError( llvm::formatv("No compile command for {0}", ModuleUnitFileName)); - for (auto &RequiredModuleName : MDB.getRequiredModules(ModuleUnitFileName)) { kadircet wrote: i am a little worried about this change. previously we were making sure all the dependent modules were built within this free function. now we're relying on the caller ensuring that, without any checks. can we keep this behavior, but pass in the module-cache as a parameter and skip building if module-file is available in the cache? later on caller can update the cache using the information in `BuiltModuleFiles`. https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
@@ -228,54 +223,30 @@ class StandalonePrerequisiteModules : public PrerequisiteModules { return BuiltModuleNames.contains(ModuleName); } - void addModuleFile(llvm::StringRef ModuleName, - llvm::StringRef ModuleFilePath) { -RequiredModules.emplace_back(ModuleName, ModuleFilePath); -BuiltModuleNames.insert(ModuleName); + void addModuleFile(std::shared_ptr BMI) { +BuiltModuleNames.insert(BMI->ModuleName); +RequiredModules.emplace_back(std::move(BMI)); } private: - llvm::SmallVector RequiredModules; + mutable llvm::SmallVector, 8> RequiredModules; kadircet wrote: why do we need to make this mutable? also let's store a `std::shared_ptr` instead (and update rest of the places to make sure a module file is never mutated after construction) https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
@@ -482,6 +482,42 @@ void func() { EXPECT_EQ(Result.signatures[0].parameters[0].labelString, "int a"); } +TEST_F(PrerequisiteModulesTests, ReusablePrerequisiteModulesTest) { kadircet wrote: can you also add some tests demonstrating behavior in presence of stale cache entries? https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
@@ -316,36 +287,169 @@ llvm::Error buildModuleFile(llvm::StringRef ModuleName, if (Clang->getDiagnostics().hasErrorOccurred()) return llvm::createStringError("Compilation failed"); - BuiltModuleFiles.addModuleFile(ModuleName, Inputs.CompileCommand.Output); - return llvm::Error::success(); + return ModuleFile{ModuleName, Inputs.CompileCommand.Output}; +} + +bool ReusablePrerequisiteModules::canReuse( +const CompilerInvocation &CI, +llvm::IntrusiveRefCntPtr VFS) const { + if (RequiredModules.empty()) +return true; + + SmallVector BMIPaths; + for (auto &MF : RequiredModules) +BMIPaths.push_back(MF->ModuleFilePath); + return IsModuleFilesUpToDate(BMIPaths, *this, VFS); } } // namespace +class ModulesBuilder::ModuleFileCache { +public: + ModuleFileCache(const GlobalCompilationDatabase &CDB) : CDB(CDB) {} + + llvm::Error + getOrBuildModuleFile(StringRef ModuleName, const ThreadsafeFS &TFS, + ProjectModules &MDB, + ReusablePrerequisiteModules &RequiredModules); + const GlobalCompilationDatabase &getCDB() const { return CDB; } + +private: + std::shared_ptr + getValidModuleFile(StringRef ModuleName, ProjectModules &MDB, + const ThreadsafeFS &TFS, + PrerequisiteModules &BuiltModuleFiles); + + /// This should only be called by getValidModuleFile. This is unlocked version + /// of getValidModuleFile. The function is extracted to avoid dead locks when + /// recursing. + std::shared_ptr + isValidModuleFileUnlocked(StringRef ModuleName, ProjectModules &MDB, +const ThreadsafeFS &TFS, +PrerequisiteModules &BuiltModuleFiles); + + const GlobalCompilationDatabase &CDB; + + llvm::StringMap> ModuleFiles; + // Mutex to guard accesses to ModuleFiles. + std::mutex ModuleFilesMutex; +}; + +std::shared_ptr +ModulesBuilder::ModuleFileCache::isValidModuleFileUnlocked( kadircet wrote: doing all of this work while holding a lock makes me nervous. what about: - first perform the traversal to find all the relevant module names, without holding any locks - then acquire the lock, check if all module names exist in the cache and also copy the modulefiles and release the lock - now we can check if all the modulefiles are up-to-date and return accordingly. https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
@@ -316,36 +287,169 @@ llvm::Error buildModuleFile(llvm::StringRef ModuleName, if (Clang->getDiagnostics().hasErrorOccurred()) return llvm::createStringError("Compilation failed"); - BuiltModuleFiles.addModuleFile(ModuleName, Inputs.CompileCommand.Output); - return llvm::Error::success(); + return ModuleFile{ModuleName, Inputs.CompileCommand.Output}; +} + +bool ReusablePrerequisiteModules::canReuse( +const CompilerInvocation &CI, +llvm::IntrusiveRefCntPtr VFS) const { + if (RequiredModules.empty()) +return true; + + SmallVector BMIPaths; + for (auto &MF : RequiredModules) +BMIPaths.push_back(MF->ModuleFilePath); + return IsModuleFilesUpToDate(BMIPaths, *this, VFS); } } // namespace +class ModulesBuilder::ModuleFileCache { +public: + ModuleFileCache(const GlobalCompilationDatabase &CDB) : CDB(CDB) {} + + llvm::Error + getOrBuildModuleFile(StringRef ModuleName, const ThreadsafeFS &TFS, + ProjectModules &MDB, + ReusablePrerequisiteModules &RequiredModules); + const GlobalCompilationDatabase &getCDB() const { return CDB; } + +private: + std::shared_ptr + getValidModuleFile(StringRef ModuleName, ProjectModules &MDB, + const ThreadsafeFS &TFS, + PrerequisiteModules &BuiltModuleFiles); + + /// This should only be called by getValidModuleFile. This is unlocked version + /// of getValidModuleFile. The function is extracted to avoid dead locks when + /// recursing. + std::shared_ptr + isValidModuleFileUnlocked(StringRef ModuleName, ProjectModules &MDB, +const ThreadsafeFS &TFS, +PrerequisiteModules &BuiltModuleFiles); + + const GlobalCompilationDatabase &CDB; + + llvm::StringMap> ModuleFiles; kadircet wrote: can we keep a std::weak_ptr here instead? That way we can ensure that a module-file is kept alive iff there's a prerequisitemodules referencing it. we're still left with this map growing unboundedly, but map entries are cheap, just a pointer and a modulename. that can also be addressed by triggering a cleanup when we destroy a prerequisitemodules. https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
https://github.com/kadircet edited https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
@@ -316,36 +287,169 @@ llvm::Error buildModuleFile(llvm::StringRef ModuleName, if (Clang->getDiagnostics().hasErrorOccurred()) return llvm::createStringError("Compilation failed"); - BuiltModuleFiles.addModuleFile(ModuleName, Inputs.CompileCommand.Output); - return llvm::Error::success(); + return ModuleFile{ModuleName, Inputs.CompileCommand.Output}; +} + +bool ReusablePrerequisiteModules::canReuse( +const CompilerInvocation &CI, +llvm::IntrusiveRefCntPtr VFS) const { + if (RequiredModules.empty()) +return true; + + SmallVector BMIPaths; + for (auto &MF : RequiredModules) +BMIPaths.push_back(MF->ModuleFilePath); + return IsModuleFilesUpToDate(BMIPaths, *this, VFS); } } // namespace +class ModulesBuilder::ModuleFileCache { +public: + ModuleFileCache(const GlobalCompilationDatabase &CDB) : CDB(CDB) {} + + llvm::Error + getOrBuildModuleFile(StringRef ModuleName, const ThreadsafeFS &TFS, + ProjectModules &MDB, + ReusablePrerequisiteModules &RequiredModules); + const GlobalCompilationDatabase &getCDB() const { return CDB; } + +private: + std::shared_ptr + getValidModuleFile(StringRef ModuleName, ProjectModules &MDB, + const ThreadsafeFS &TFS, + PrerequisiteModules &BuiltModuleFiles); + + /// This should only be called by getValidModuleFile. This is unlocked version + /// of getValidModuleFile. The function is extracted to avoid dead locks when + /// recursing. + std::shared_ptr + isValidModuleFileUnlocked(StringRef ModuleName, ProjectModules &MDB, +const ThreadsafeFS &TFS, +PrerequisiteModules &BuiltModuleFiles); + + const GlobalCompilationDatabase &CDB; + + llvm::StringMap> ModuleFiles; + // Mutex to guard accesses to ModuleFiles. + std::mutex ModuleFilesMutex; +}; + +std::shared_ptr +ModulesBuilder::ModuleFileCache::isValidModuleFileUnlocked( +StringRef ModuleName, ProjectModules &MDB, const ThreadsafeFS &TFS, +PrerequisiteModules &BuiltModuleFiles) { + auto Iter = ModuleFiles.find(ModuleName); + if (Iter != ModuleFiles.end()) { +if (!IsModuleFileUpToDate(Iter->second->ModuleFilePath, BuiltModuleFiles, + TFS.view(std::nullopt))) { + log("Found not-up-date module file {0} for module {1} in cache", + Iter->second->ModuleFilePath, ModuleName); + ModuleFiles.erase(Iter); + return nullptr; +} + +if (llvm::any_of( +MDB.getRequiredModules(MDB.getSourceForModuleName(ModuleName)), +[&MDB, &TFS, &BuiltModuleFiles, this](auto &&RequiredModuleName) { + return !isValidModuleFileUnlocked(RequiredModuleName, MDB, TFS, +BuiltModuleFiles); +})) { + ModuleFiles.erase(Iter); + return nullptr; +} + +return Iter->second; + } + + log("Don't find {0} in cache", ModuleName); + + return nullptr; +} + +std::shared_ptr ModulesBuilder::ModuleFileCache::getValidModuleFile( +StringRef ModuleName, ProjectModules &MDB, const ThreadsafeFS &TFS, +PrerequisiteModules &BuiltModuleFiles) { + std::lock_guard _(ModuleFilesMutex); + + return isValidModuleFileUnlocked(ModuleName, MDB, TFS, BuiltModuleFiles); +} + +llvm::Error ModulesBuilder::ModuleFileCache::getOrBuildModuleFile( kadircet wrote: similar to comment in `buildModuleFile`, can we turn this into just `getModuleFile`, which returns the latest cached module-file and make it callers responsibility to deal with freshness of the data. I don't think making this class concerned about both cache management and building/verifying module-files is necessary (and goes against separation of concerns). https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
https://github.com/kadircet requested changes to this pull request. thanks I think this LG in terms of module-builder interfaces, but I think we can make some more changes to implementation to ensure it's easier to maintain going forward. speaking of maintenance, @HighCommander4 is definitely doing more of that than me recently. so i'd like to make sure he's also ok with this new complexity. i also would like to highlight @ChuanqiXu9 is definitely a responsible member of community, hence i believe he'll also be around if we have bug reports about this bits. https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
https://github.com/ChuanqiXu9 edited https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
@@ -338,17 +460,129 @@ ModulesBuilder::buildPrerequisiteModulesFor(PathRef File, return std::move(RequiredModules); } -bool StandalonePrerequisiteModules::canReuse( +ReusableModulesBuilder::ModuleBuildingSharedOwner +ReusableModulesBuilder::getOrCreateModuleBuildingOwner(StringRef ModuleName) { + std::lock_guard _(ModulesBuildingMutex); + + auto MutexIter = BuildingModuleMutexes.find(ModuleName); + if (MutexIter == BuildingModuleMutexes.end()) +MutexIter = BuildingModuleMutexes +.try_emplace(ModuleName, std::make_shared()) +.first; + + auto CVIter = BuildingModuleCVs.find(ModuleName); + if (CVIter == BuildingModuleCVs.end()) +CVIter = BuildingModuleCVs + .try_emplace(ModuleName, + std::make_shared()) + .first; + + return ModuleBuildingSharedOwner(ModuleName, MutexIter->getValue(), + CVIter->getValue(), *this); +} + +llvm::Error ReusableModulesBuilder::getOrBuildModuleFile( +StringRef ModuleName, const ThreadsafeFS &TFS, ProjectModules &MDB, +ReusablePrerequisiteModules &BuiltModuleFiles) { + if (BuiltModuleFiles.isModuleUnitBuilt(ModuleName)) +return llvm::Error::success(); + + PathRef ModuleUnitFileName = MDB.getSourceForModuleName(ModuleName); + /// It is possible that we're meeting third party modules (modules whose + /// source are not in the project. e.g, the std module may be a third-party + /// module for most project) or something wrong with the implementation of + /// ProjectModules. + /// FIXME: How should we treat third party modules here? If we want to ignore + /// third party modules, we should return true instead of false here. + /// Currently we simply bail out. + if (ModuleUnitFileName.empty()) +return llvm::createStringError( +llvm::formatv("Don't get the module unit for module {0}", ModuleName)); + + for (auto &RequiredModuleName : MDB.getRequiredModules(ModuleUnitFileName)) +// Return early if there are errors building the module file. +if (!getOrBuildModuleFile(RequiredModuleName, TFS, MDB, BuiltModuleFiles)) + return llvm::createStringError( + llvm::formatv("Failed to build module {0}", RequiredModuleName)); + + if (std::shared_ptr Cached = + getValidModuleFile(ModuleName, MDB, TFS, BuiltModuleFiles)) { +log("Reusing module {0} from {1}", ModuleName, Cached->ModuleFilePath); +BuiltModuleFiles.addModuleFile(Cached); +return llvm::Error::success(); + } + + ModuleBuildingSharedOwner ModuleBuildingOwner = + getOrCreateModuleBuildingOwner(ModuleName); + + std::condition_variable &CV = ModuleBuildingOwner.getCV(); + std::unique_lock lk(ModuleBuildingOwner.getMutex()); + if (!ModuleBuildingOwner.isUniqueBuildingOwner()) { ChuanqiXu9 wrote: Done. Sounds not bad. https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
https://github.com/ChuanqiXu9 updated https://github.com/llvm/llvm-project/pull/106683 >From 15aa6af1f5d22e4b837e8e2fd49469310ffbe7f1 Mon Sep 17 00:00:00 2001 From: Chuanqi Xu Date: Fri, 30 Aug 2024 15:11:07 +0800 Subject: [PATCH 1/2] [clangd] [Modules] Support Reusable Modules Builder --- clang-tools-extra/clangd/ModulesBuilder.cpp | 375 ++ clang-tools-extra/clangd/ModulesBuilder.h | 8 +- .../unittests/PrerequisiteModulesTest.cpp | 36 ++ 3 files changed, 344 insertions(+), 75 deletions(-) diff --git a/clang-tools-extra/clangd/ModulesBuilder.cpp b/clang-tools-extra/clangd/ModulesBuilder.cpp index 97f67ddf5495a2..c4fb3d4010d370 100644 --- a/clang-tools-extra/clangd/ModulesBuilder.cpp +++ b/clang-tools-extra/clangd/ModulesBuilder.cpp @@ -13,6 +13,7 @@ #include "clang/Frontend/FrontendActions.h" #include "clang/Serialization/ASTReader.h" #include "clang/Serialization/InMemoryModuleCache.h" +#include "llvm/ADT/ScopeExit.h" namespace clang { namespace clangd { @@ -192,33 +193,27 @@ bool IsModuleFilesUpToDate( }); } -// StandalonePrerequisiteModules - stands for PrerequisiteModules for which all +// ReusablePrerequisiteModules - stands for PrerequisiteModules for which all // the required modules are built successfully. All the module files -// are owned by the StandalonePrerequisiteModules class. -// -// Any of the built module files won't be shared with other instances of the -// class. So that we can avoid worrying thread safety. -// -// We don't need to worry about duplicated module names here since the standard -// guarantees the module names should be unique to a program. -class StandalonePrerequisiteModules : public PrerequisiteModules { +// are owned by the modules builder. +class ReusablePrerequisiteModules : public PrerequisiteModules { public: - StandalonePrerequisiteModules() = default; + ReusablePrerequisiteModules() = default; - StandalonePrerequisiteModules(const StandalonePrerequisiteModules &) = delete; - StandalonePrerequisiteModules - operator=(const StandalonePrerequisiteModules &) = delete; - StandalonePrerequisiteModules(StandalonePrerequisiteModules &&) = delete; - StandalonePrerequisiteModules - operator=(StandalonePrerequisiteModules &&) = delete; + ReusablePrerequisiteModules(const ReusablePrerequisiteModules &) = delete; + ReusablePrerequisiteModules + operator=(const ReusablePrerequisiteModules &) = delete; + ReusablePrerequisiteModules(ReusablePrerequisiteModules &&) = delete; + ReusablePrerequisiteModules + operator=(ReusablePrerequisiteModules &&) = delete; - ~StandalonePrerequisiteModules() override = default; + ~ReusablePrerequisiteModules() override = default; void adjustHeaderSearchOptions(HeaderSearchOptions &Options) const override { // Appending all built module files. for (auto &RequiredModule : RequiredModules) Options.PrebuiltModuleFiles.insert_or_assign( - RequiredModule.ModuleName, RequiredModule.ModuleFilePath); + RequiredModule->ModuleName, RequiredModule->ModuleFilePath); } bool canReuse(const CompilerInvocation &CI, @@ -228,54 +223,30 @@ class StandalonePrerequisiteModules : public PrerequisiteModules { return BuiltModuleNames.contains(ModuleName); } - void addModuleFile(llvm::StringRef ModuleName, - llvm::StringRef ModuleFilePath) { -RequiredModules.emplace_back(ModuleName, ModuleFilePath); -BuiltModuleNames.insert(ModuleName); + void addModuleFile(std::shared_ptr BMI) { +BuiltModuleNames.insert(BMI->ModuleName); +RequiredModules.emplace_back(std::move(BMI)); } private: - llvm::SmallVector RequiredModules; + mutable llvm::SmallVector, 8> RequiredModules; // A helper class to speedup the query if a module is built. llvm::StringSet<> BuiltModuleNames; }; -// Build a module file for module with `ModuleName`. The information of built -// module file are stored in \param BuiltModuleFiles. -llvm::Error buildModuleFile(llvm::StringRef ModuleName, -const GlobalCompilationDatabase &CDB, -const ThreadsafeFS &TFS, ProjectModules &MDB, -PathRef ModuleFilesPrefix, -StandalonePrerequisiteModules &BuiltModuleFiles) { - if (BuiltModuleFiles.isModuleUnitBuilt(ModuleName)) -return llvm::Error::success(); - - PathRef ModuleUnitFileName = MDB.getSourceForModuleName(ModuleName); - // It is possible that we're meeting third party modules (modules whose - // source are not in the project. e.g, the std module may be a third-party - // module for most projects) or something wrong with the implementation of - // ProjectModules. - // FIXME: How should we treat third party modules here? If we want to ignore - // third party modules, we should return true instead of false here. - // Currently we simply bail out. - if (ModuleUnitFileName.empty()) -return llvm::createStringErro
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
@@ -85,19 +85,20 @@ class PrerequisiteModules { /// different versions and different source files. class ModulesBuilder { ChuanqiXu9 wrote: Done https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
@@ -85,19 +85,20 @@ class PrerequisiteModules { /// different versions and different source files. class ModulesBuilder { public: - ModulesBuilder(const GlobalCompilationDatabase &CDB) : CDB(CDB) {} + ModulesBuilder() = default; + virtual ~ModulesBuilder() = default; ModulesBuilder(const ModulesBuilder &) = delete; ModulesBuilder(ModulesBuilder &&) = delete; ModulesBuilder &operator=(const ModulesBuilder &) = delete; ModulesBuilder &operator=(ModulesBuilder &&) = delete; - std::unique_ptr - buildPrerequisiteModulesFor(PathRef File, const ThreadsafeFS &TFS) const; + virtual std::unique_ptr ChuanqiXu9 wrote: Yes. This should be covered by another comment. https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
https://github.com/ChuanqiXu9 updated https://github.com/llvm/llvm-project/pull/106683 >From 15aa6af1f5d22e4b837e8e2fd49469310ffbe7f1 Mon Sep 17 00:00:00 2001 From: Chuanqi Xu Date: Fri, 30 Aug 2024 15:11:07 +0800 Subject: [PATCH] [clangd] [Modules] Support Reusable Modules Builder --- clang-tools-extra/clangd/ModulesBuilder.cpp | 375 ++ clang-tools-extra/clangd/ModulesBuilder.h | 8 +- .../unittests/PrerequisiteModulesTest.cpp | 36 ++ 3 files changed, 344 insertions(+), 75 deletions(-) diff --git a/clang-tools-extra/clangd/ModulesBuilder.cpp b/clang-tools-extra/clangd/ModulesBuilder.cpp index 97f67ddf5495a2..c4fb3d4010d370 100644 --- a/clang-tools-extra/clangd/ModulesBuilder.cpp +++ b/clang-tools-extra/clangd/ModulesBuilder.cpp @@ -13,6 +13,7 @@ #include "clang/Frontend/FrontendActions.h" #include "clang/Serialization/ASTReader.h" #include "clang/Serialization/InMemoryModuleCache.h" +#include "llvm/ADT/ScopeExit.h" namespace clang { namespace clangd { @@ -192,33 +193,27 @@ bool IsModuleFilesUpToDate( }); } -// StandalonePrerequisiteModules - stands for PrerequisiteModules for which all +// ReusablePrerequisiteModules - stands for PrerequisiteModules for which all // the required modules are built successfully. All the module files -// are owned by the StandalonePrerequisiteModules class. -// -// Any of the built module files won't be shared with other instances of the -// class. So that we can avoid worrying thread safety. -// -// We don't need to worry about duplicated module names here since the standard -// guarantees the module names should be unique to a program. -class StandalonePrerequisiteModules : public PrerequisiteModules { +// are owned by the modules builder. +class ReusablePrerequisiteModules : public PrerequisiteModules { public: - StandalonePrerequisiteModules() = default; + ReusablePrerequisiteModules() = default; - StandalonePrerequisiteModules(const StandalonePrerequisiteModules &) = delete; - StandalonePrerequisiteModules - operator=(const StandalonePrerequisiteModules &) = delete; - StandalonePrerequisiteModules(StandalonePrerequisiteModules &&) = delete; - StandalonePrerequisiteModules - operator=(StandalonePrerequisiteModules &&) = delete; + ReusablePrerequisiteModules(const ReusablePrerequisiteModules &) = delete; + ReusablePrerequisiteModules + operator=(const ReusablePrerequisiteModules &) = delete; + ReusablePrerequisiteModules(ReusablePrerequisiteModules &&) = delete; + ReusablePrerequisiteModules + operator=(ReusablePrerequisiteModules &&) = delete; - ~StandalonePrerequisiteModules() override = default; + ~ReusablePrerequisiteModules() override = default; void adjustHeaderSearchOptions(HeaderSearchOptions &Options) const override { // Appending all built module files. for (auto &RequiredModule : RequiredModules) Options.PrebuiltModuleFiles.insert_or_assign( - RequiredModule.ModuleName, RequiredModule.ModuleFilePath); + RequiredModule->ModuleName, RequiredModule->ModuleFilePath); } bool canReuse(const CompilerInvocation &CI, @@ -228,54 +223,30 @@ class StandalonePrerequisiteModules : public PrerequisiteModules { return BuiltModuleNames.contains(ModuleName); } - void addModuleFile(llvm::StringRef ModuleName, - llvm::StringRef ModuleFilePath) { -RequiredModules.emplace_back(ModuleName, ModuleFilePath); -BuiltModuleNames.insert(ModuleName); + void addModuleFile(std::shared_ptr BMI) { +BuiltModuleNames.insert(BMI->ModuleName); +RequiredModules.emplace_back(std::move(BMI)); } private: - llvm::SmallVector RequiredModules; + mutable llvm::SmallVector, 8> RequiredModules; // A helper class to speedup the query if a module is built. llvm::StringSet<> BuiltModuleNames; }; -// Build a module file for module with `ModuleName`. The information of built -// module file are stored in \param BuiltModuleFiles. -llvm::Error buildModuleFile(llvm::StringRef ModuleName, -const GlobalCompilationDatabase &CDB, -const ThreadsafeFS &TFS, ProjectModules &MDB, -PathRef ModuleFilesPrefix, -StandalonePrerequisiteModules &BuiltModuleFiles) { - if (BuiltModuleFiles.isModuleUnitBuilt(ModuleName)) -return llvm::Error::success(); - - PathRef ModuleUnitFileName = MDB.getSourceForModuleName(ModuleName); - // It is possible that we're meeting third party modules (modules whose - // source are not in the project. e.g, the std module may be a third-party - // module for most projects) or something wrong with the implementation of - // ProjectModules. - // FIXME: How should we treat third party modules here? If we want to ignore - // third party modules, we should return true instead of false here. - // Currently we simply bail out. - if (ModuleUnitFileName.empty()) -return llvm::createStringError("F
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
@@ -85,19 +85,20 @@ class PrerequisiteModules { /// different versions and different source files. class ModulesBuilder { kadircet wrote: > I was told we don't like forward declaration in clangd Yes in theory this is also a forward declaration, but it's not in the same bucket as others. It's here to hide an implementation detail. Since the class is declared in a private section and implemented in the CC file, no user can actually see a definition. > And if we can do that, I like the style too. glad to hear that. i think this should simplify the changes a lot! https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
@@ -338,17 +460,129 @@ ModulesBuilder::buildPrerequisiteModulesFor(PathRef File, return std::move(RequiredModules); } -bool StandalonePrerequisiteModules::canReuse( +ReusableModulesBuilder::ModuleBuildingSharedOwner +ReusableModulesBuilder::getOrCreateModuleBuildingOwner(StringRef ModuleName) { + std::lock_guard _(ModulesBuildingMutex); + + auto MutexIter = BuildingModuleMutexes.find(ModuleName); + if (MutexIter == BuildingModuleMutexes.end()) +MutexIter = BuildingModuleMutexes +.try_emplace(ModuleName, std::make_shared()) +.first; + + auto CVIter = BuildingModuleCVs.find(ModuleName); + if (CVIter == BuildingModuleCVs.end()) +CVIter = BuildingModuleCVs + .try_emplace(ModuleName, + std::make_shared()) + .first; + + return ModuleBuildingSharedOwner(ModuleName, MutexIter->getValue(), + CVIter->getValue(), *this); +} + +llvm::Error ReusableModulesBuilder::getOrBuildModuleFile( +StringRef ModuleName, const ThreadsafeFS &TFS, ProjectModules &MDB, +ReusablePrerequisiteModules &BuiltModuleFiles) { + if (BuiltModuleFiles.isModuleUnitBuilt(ModuleName)) +return llvm::Error::success(); + + PathRef ModuleUnitFileName = MDB.getSourceForModuleName(ModuleName); + /// It is possible that we're meeting third party modules (modules whose + /// source are not in the project. e.g, the std module may be a third-party + /// module for most project) or something wrong with the implementation of + /// ProjectModules. + /// FIXME: How should we treat third party modules here? If we want to ignore + /// third party modules, we should return true instead of false here. + /// Currently we simply bail out. + if (ModuleUnitFileName.empty()) +return llvm::createStringError( +llvm::formatv("Don't get the module unit for module {0}", ModuleName)); + + for (auto &RequiredModuleName : MDB.getRequiredModules(ModuleUnitFileName)) +// Return early if there are errors building the module file. +if (!getOrBuildModuleFile(RequiredModuleName, TFS, MDB, BuiltModuleFiles)) + return llvm::createStringError( + llvm::formatv("Failed to build module {0}", RequiredModuleName)); + + if (std::shared_ptr Cached = + getValidModuleFile(ModuleName, MDB, TFS, BuiltModuleFiles)) { +log("Reusing module {0} from {1}", ModuleName, Cached->ModuleFilePath); +BuiltModuleFiles.addModuleFile(Cached); +return llvm::Error::success(); + } + + ModuleBuildingSharedOwner ModuleBuildingOwner = + getOrCreateModuleBuildingOwner(ModuleName); + + std::condition_variable &CV = ModuleBuildingOwner.getCV(); + std::unique_lock lk(ModuleBuildingOwner.getMutex()); + if (!ModuleBuildingOwner.isUniqueBuildingOwner()) { kadircet wrote: i am definitely not opposed to the idea, i just think we can land it in a separate change and make this PR much easier to review. https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
@@ -85,19 +85,20 @@ class PrerequisiteModules { /// different versions and different source files. class ModulesBuilder { public: - ModulesBuilder(const GlobalCompilationDatabase &CDB) : CDB(CDB) {} + ModulesBuilder() = default; + virtual ~ModulesBuilder() = default; ModulesBuilder(const ModulesBuilder &) = delete; ModulesBuilder(ModulesBuilder &&) = delete; ModulesBuilder &operator=(const ModulesBuilder &) = delete; ModulesBuilder &operator=(ModulesBuilder &&) = delete; - std::unique_ptr - buildPrerequisiteModulesFor(PathRef File, const ThreadsafeFS &TFS) const; + virtual std::unique_ptr kadircet wrote: >Since I want to avoid introducing the complex date members for >ReusableModulesBuilder into the header file. I feel the current codes may be >more neat. I totally agree with that. I think the recommendations in the rest of the comments mitigates this concern. Especially the one around just declaring a caching-class as an implementation detail and not polluting the header with any other details. https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
@@ -338,17 +460,129 @@ ModulesBuilder::buildPrerequisiteModulesFor(PathRef File, return std::move(RequiredModules); } -bool StandalonePrerequisiteModules::canReuse( +ReusableModulesBuilder::ModuleBuildingSharedOwner +ReusableModulesBuilder::getOrCreateModuleBuildingOwner(StringRef ModuleName) { + std::lock_guard _(ModulesBuildingMutex); + + auto MutexIter = BuildingModuleMutexes.find(ModuleName); + if (MutexIter == BuildingModuleMutexes.end()) +MutexIter = BuildingModuleMutexes +.try_emplace(ModuleName, std::make_shared()) +.first; + + auto CVIter = BuildingModuleCVs.find(ModuleName); + if (CVIter == BuildingModuleCVs.end()) +CVIter = BuildingModuleCVs + .try_emplace(ModuleName, + std::make_shared()) + .first; + + return ModuleBuildingSharedOwner(ModuleName, MutexIter->getValue(), + CVIter->getValue(), *this); +} + +llvm::Error ReusableModulesBuilder::getOrBuildModuleFile( +StringRef ModuleName, const ThreadsafeFS &TFS, ProjectModules &MDB, +ReusablePrerequisiteModules &BuiltModuleFiles) { + if (BuiltModuleFiles.isModuleUnitBuilt(ModuleName)) +return llvm::Error::success(); + + PathRef ModuleUnitFileName = MDB.getSourceForModuleName(ModuleName); + /// It is possible that we're meeting third party modules (modules whose + /// source are not in the project. e.g, the std module may be a third-party + /// module for most project) or something wrong with the implementation of + /// ProjectModules. + /// FIXME: How should we treat third party modules here? If we want to ignore + /// third party modules, we should return true instead of false here. + /// Currently we simply bail out. + if (ModuleUnitFileName.empty()) +return llvm::createStringError( +llvm::formatv("Don't get the module unit for module {0}", ModuleName)); + + for (auto &RequiredModuleName : MDB.getRequiredModules(ModuleUnitFileName)) +// Return early if there are errors building the module file. +if (!getOrBuildModuleFile(RequiredModuleName, TFS, MDB, BuiltModuleFiles)) + return llvm::createStringError( + llvm::formatv("Failed to build module {0}", RequiredModuleName)); + + if (std::shared_ptr Cached = + getValidModuleFile(ModuleName, MDB, TFS, BuiltModuleFiles)) { +log("Reusing module {0} from {1}", ModuleName, Cached->ModuleFilePath); +BuiltModuleFiles.addModuleFile(Cached); +return llvm::Error::success(); + } + + ModuleBuildingSharedOwner ModuleBuildingOwner = + getOrCreateModuleBuildingOwner(ModuleName); + + std::condition_variable &CV = ModuleBuildingOwner.getCV(); + std::unique_lock lk(ModuleBuildingOwner.getMutex()); + if (!ModuleBuildingOwner.isUniqueBuildingOwner()) { ChuanqiXu9 wrote: I prefer to leave it as is. Otherwise, two opened file may not be able to share the same BMI that meant to be shared. I feel it is the functionality of the patch. https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
@@ -85,19 +85,20 @@ class PrerequisiteModules { /// different versions and different source files. class ModulesBuilder { ChuanqiXu9 wrote: I didn't do it since in the last review, I was told we don't like forward declaration in clangd. And if we can do that, I like the style too. https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
@@ -85,19 +85,20 @@ class PrerequisiteModules { /// different versions and different source files. class ModulesBuilder { public: - ModulesBuilder(const GlobalCompilationDatabase &CDB) : CDB(CDB) {} + ModulesBuilder() = default; + virtual ~ModulesBuilder() = default; ModulesBuilder(const ModulesBuilder &) = delete; ModulesBuilder(ModulesBuilder &&) = delete; ModulesBuilder &operator=(const ModulesBuilder &) = delete; ModulesBuilder &operator=(ModulesBuilder &&) = delete; - std::unique_ptr - buildPrerequisiteModulesFor(PathRef File, const ThreadsafeFS &TFS) const; + virtual std::unique_ptr ChuanqiXu9 wrote: Since I want to avoid introducing the complex date members for `ReusableModulesBuilder` into the header file. I feel the current codes may be more neat. Would you like to move that into the header? https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
https://github.com/kadircet requested changes to this pull request. https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
@@ -338,17 +460,129 @@ ModulesBuilder::buildPrerequisiteModulesFor(PathRef File, return std::move(RequiredModules); } -bool StandalonePrerequisiteModules::canReuse( +ReusableModulesBuilder::ModuleBuildingSharedOwner +ReusableModulesBuilder::getOrCreateModuleBuildingOwner(StringRef ModuleName) { + std::lock_guard _(ModulesBuildingMutex); + + auto MutexIter = BuildingModuleMutexes.find(ModuleName); + if (MutexIter == BuildingModuleMutexes.end()) +MutexIter = BuildingModuleMutexes +.try_emplace(ModuleName, std::make_shared()) +.first; + + auto CVIter = BuildingModuleCVs.find(ModuleName); + if (CVIter == BuildingModuleCVs.end()) +CVIter = BuildingModuleCVs + .try_emplace(ModuleName, + std::make_shared()) + .first; + + return ModuleBuildingSharedOwner(ModuleName, MutexIter->getValue(), + CVIter->getValue(), *this); +} + +llvm::Error ReusableModulesBuilder::getOrBuildModuleFile( +StringRef ModuleName, const ThreadsafeFS &TFS, ProjectModules &MDB, +ReusablePrerequisiteModules &BuiltModuleFiles) { + if (BuiltModuleFiles.isModuleUnitBuilt(ModuleName)) +return llvm::Error::success(); + + PathRef ModuleUnitFileName = MDB.getSourceForModuleName(ModuleName); + /// It is possible that we're meeting third party modules (modules whose + /// source are not in the project. e.g, the std module may be a third-party + /// module for most project) or something wrong with the implementation of + /// ProjectModules. + /// FIXME: How should we treat third party modules here? If we want to ignore + /// third party modules, we should return true instead of false here. + /// Currently we simply bail out. + if (ModuleUnitFileName.empty()) +return llvm::createStringError( +llvm::formatv("Don't get the module unit for module {0}", ModuleName)); + + for (auto &RequiredModuleName : MDB.getRequiredModules(ModuleUnitFileName)) +// Return early if there are errors building the module file. +if (!getOrBuildModuleFile(RequiredModuleName, TFS, MDB, BuiltModuleFiles)) + return llvm::createStringError( + llvm::formatv("Failed to build module {0}", RequiredModuleName)); + + if (std::shared_ptr Cached = + getValidModuleFile(ModuleName, MDB, TFS, BuiltModuleFiles)) { +log("Reusing module {0} from {1}", ModuleName, Cached->ModuleFilePath); +BuiltModuleFiles.addModuleFile(Cached); +return llvm::Error::success(); + } + + ModuleBuildingSharedOwner ModuleBuildingOwner = + getOrCreateModuleBuildingOwner(ModuleName); + + std::condition_variable &CV = ModuleBuildingOwner.getCV(); + std::unique_lock lk(ModuleBuildingOwner.getMutex()); + if (!ModuleBuildingOwner.isUniqueBuildingOwner()) { kadircet wrote: this sounds like a really nice optimization, but seems to be complicating the logic a lot. can we land this in a follow up change ? https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
@@ -85,19 +85,20 @@ class PrerequisiteModules { /// different versions and different source files. class ModulesBuilder { public: - ModulesBuilder(const GlobalCompilationDatabase &CDB) : CDB(CDB) {} + ModulesBuilder() = default; + virtual ~ModulesBuilder() = default; ModulesBuilder(const ModulesBuilder &) = delete; ModulesBuilder(ModulesBuilder &&) = delete; ModulesBuilder &operator=(const ModulesBuilder &) = delete; ModulesBuilder &operator=(ModulesBuilder &&) = delete; - std::unique_ptr - buildPrerequisiteModulesFor(PathRef File, const ThreadsafeFS &TFS) const; + virtual std::unique_ptr kadircet wrote: not sure why we need to make this an abstract base class, instead of concretely implementing the functionality here. we're only using the caching version of `ModulesBuilder` in actual code & tests anyway. https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
@@ -85,19 +85,20 @@ class PrerequisiteModules { /// different versions and different source files. class ModulesBuilder { kadircet wrote: can we just introduce a new helper class ```cpp class ModulesBuilder { ... private: class ModuleFileCache; std::unique_ptr MFCache; }; ``` in `ModulesBuilder.cpp`: ```cpp class ModuleFileCache { private: std::mutex CacheMu; StringMap> Cache; public: // Returns cahed `ModuleName` if one exists and it's up-to-date per BuiltModuleFiles. // It's a prereq for BuiltModuleFiles to contain all the prereqs of ModuleName (i.e. this function is not recursive) std::shared_ptr getOrBuildModule(llvm::StringRef ModuleName, const GlobalCompilationDatabase &CDB, const ThreadsafeFS &TFS, ProjectModules &MDB, PathRef ModuleFilesPrefix, StandalonePrerequisiteModules &BuiltModuleFiles); }; ``` Afterwards we can keep most of the logic as-is. we just need to update `buildModuleFile` to take in a `ModuleFileCache&` as a parameter and call `ModuleFileCache::getOrBuildModule` instead of building one itself. We also need to verify freshness of a module-file we're going to return from cache in `getOrBuildModule`, we can do so efficiently by only verifying the sources that are directly part of `ModuleName` and not the transitive closure (we can assume all the deps were already checked when putting `BuiltModuleFiles` together). You can store a pointer to `ModuleFileCache` in `ModuleFile` to trigger deletion from map on destruction of `ModuleFile`s. https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
ChuanqiXu9 wrote: I'd like to land this in 2 weeks if no more comments came in. Given: - In our downstream, we've landed this patch for more than a year and it seems running well. And also in the open source world, I tried to send it to https://github.com/ChuanqiXu9/clangd-for-modules . Everyone experienced it didn't complain about it for bugs. So I believe this is well tested. - There are other following patches and I really wish we can have something workable in the next release. - This patch doesn't change the shape of modules support in clangd. The major change lives in ModulesBuilder.cpp, which we can think it as an implementation detail instead of some global things. For policy related comments, please sent it to https://discourse.llvm.org/t/rfc-directions-for-modules-support-in-clangd/78072/6 https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
ChuanqiXu9 wrote: @kadircet ping~ It will be pretty good for the users if we can have this for clang20. https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
ChuanqiXu9 wrote: @sam-mccall @kadircet @HighCommander4 ping https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
ChuanqiXu9 wrote: @sam-mccall @kadircet @HighCommander4 ping~ https://github.com/llvm/llvm-project/pull/106683 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)
https://github.com/ChuanqiXu9 updated https://github.com/llvm/llvm-project/pull/106683 >From facbe53b41f90d2b7c94d83aaaec03f83f355735 Mon Sep 17 00:00:00 2001 From: Chuanqi Xu Date: Fri, 30 Aug 2024 15:11:07 +0800 Subject: [PATCH] [clangd] [Modules] Support Reusable Modules Builder --- clang-tools-extra/clangd/ClangdLSPServer.cpp | 4 +- clang-tools-extra/clangd/ClangdLSPServer.h| 2 +- clang-tools-extra/clangd/ModulesBuilder.cpp | 364 ++ clang-tools-extra/clangd/ModulesBuilder.h | 11 +- clang-tools-extra/clangd/tool/Check.cpp | 6 +- .../unittests/PrerequisiteModulesTest.cpp | 70 +++- 6 files changed, 367 insertions(+), 90 deletions(-) diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp b/clang-tools-extra/clangd/ClangdLSPServer.cpp index 06573a57554245..53a18bd4ecd1a7 100644 --- a/clang-tools-extra/clangd/ClangdLSPServer.cpp +++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp @@ -567,8 +567,8 @@ void ClangdLSPServer::onInitialize(const InitializeParams &Params, std::move(Mangler)); if (Opts.EnableExperimentalModulesSupport) { -ModulesManager.emplace(*CDB); -Opts.ModulesManager = &*ModulesManager; +ModulesManager = ModulesBuilder::getModulesBuilder(*CDB); +Opts.ModulesManager = ModulesManager.get(); } { diff --git a/clang-tools-extra/clangd/ClangdLSPServer.h b/clang-tools-extra/clangd/ClangdLSPServer.h index 0b8e4720f53236..4b8f048b430fe9 100644 --- a/clang-tools-extra/clangd/ClangdLSPServer.h +++ b/clang-tools-extra/clangd/ClangdLSPServer.h @@ -327,7 +327,7 @@ class ClangdLSPServer : private ClangdServer::Callbacks, // The ClangdServer is created by the "initialize" LSP method. std::optional Server; // Manages to build module files. - std::optional ModulesManager; + std::unique_ptr ModulesManager; }; } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/ModulesBuilder.cpp b/clang-tools-extra/clangd/ModulesBuilder.cpp index 1eeff468ef1236..a674e68ca7809d 100644 --- a/clang-tools-extra/clangd/ModulesBuilder.cpp +++ b/clang-tools-extra/clangd/ModulesBuilder.cpp @@ -12,6 +12,7 @@ #include "clang/Frontend/FrontendAction.h" #include "clang/Frontend/FrontendActions.h" #include "clang/Serialization/ASTReader.h" +#include "llvm/ADT/ScopeExit.h" namespace clang { namespace clangd { @@ -173,33 +174,27 @@ bool IsModuleFilesUpToDate( }); } -// StandalonePrerequisiteModules - stands for PrerequisiteModules for which all +// ReusablePrerequisiteModules - stands for PrerequisiteModules for which all // the required modules are built successfully. All the module files -// are owned by the StandalonePrerequisiteModules class. -// -// Any of the built module files won't be shared with other instances of the -// class. So that we can avoid worrying thread safety. -// -// We don't need to worry about duplicated module names here since the standard -// guarantees the module names should be unique to a program. -class StandalonePrerequisiteModules : public PrerequisiteModules { +// are owned by the modules builder. +class ReusablePrerequisiteModules : public PrerequisiteModules { public: - StandalonePrerequisiteModules() = default; + ReusablePrerequisiteModules() = default; - StandalonePrerequisiteModules(const StandalonePrerequisiteModules &) = delete; - StandalonePrerequisiteModules - operator=(const StandalonePrerequisiteModules &) = delete; - StandalonePrerequisiteModules(StandalonePrerequisiteModules &&) = delete; - StandalonePrerequisiteModules - operator=(StandalonePrerequisiteModules &&) = delete; + ReusablePrerequisiteModules(const ReusablePrerequisiteModules &) = delete; + ReusablePrerequisiteModules + operator=(const ReusablePrerequisiteModules &) = delete; + ReusablePrerequisiteModules(ReusablePrerequisiteModules &&) = delete; + ReusablePrerequisiteModules + operator=(ReusablePrerequisiteModules &&) = delete; - ~StandalonePrerequisiteModules() override = default; + ~ReusablePrerequisiteModules() override = default; void adjustHeaderSearchOptions(HeaderSearchOptions &Options) const override { // Appending all built module files. for (auto &RequiredModule : RequiredModules) Options.PrebuiltModuleFiles.insert_or_assign( - RequiredModule.ModuleName, RequiredModule.ModuleFilePath); + RequiredModule->ModuleName, RequiredModule->ModuleFilePath); } bool canReuse(const CompilerInvocation &CI, @@ -209,54 +204,30 @@ class StandalonePrerequisiteModules : public PrerequisiteModules { return BuiltModuleNames.contains(ModuleName); } - void addModuleFile(llvm::StringRef ModuleName, - llvm::StringRef ModuleFilePath) { -RequiredModules.emplace_back(ModuleName, ModuleFilePath); -BuiltModuleNames.insert(ModuleName); + void addModuleFile(std::shared_ptr BMI) { +BuiltModuleNames.insert(BMI->ModuleName); +RequiredModules.emplace_back(std::move(BMI)); } pr