[clang-tools-extra] [clangd] [Modules] Support Reusable Modules Builder (PR #106683)

2024-11-22 Thread Arthur Laurent via cfe-commits

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)

2024-11-12 Thread Chuanqi Xu via cfe-commits


@@ -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)

2024-11-12 Thread LLVM Continuous Integration via cfe-commits

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)

2024-11-12 Thread LLVM Continuous Integration via cfe-commits

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)

2024-11-12 Thread Chuanqi Xu via cfe-commits

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)

2024-11-12 Thread Chuanqi Xu via cfe-commits

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)

2024-11-12 Thread Chuanqi Xu via cfe-commits


@@ -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)

2024-11-12 Thread Chuanqi Xu via cfe-commits

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)

2024-11-12 Thread kadir çetinkaya via cfe-commits

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)

2024-11-12 Thread Chuanqi Xu via cfe-commits

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)

2024-11-12 Thread Chuanqi Xu via cfe-commits


@@ -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)

2024-11-12 Thread Chuanqi Xu via cfe-commits


@@ -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)

2024-11-12 Thread Chuanqi Xu via cfe-commits

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)

2024-11-12 Thread Chuanqi Xu via cfe-commits


@@ -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)

2024-11-12 Thread Chuanqi Xu via cfe-commits


@@ -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)

2024-11-12 Thread Chuanqi Xu via cfe-commits

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)

2024-11-12 Thread kadir çetinkaya via cfe-commits


@@ -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)

2024-11-12 Thread kadir çetinkaya via cfe-commits


@@ -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)

2024-11-12 Thread kadir çetinkaya via cfe-commits


@@ -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)

2024-11-12 Thread kadir çetinkaya via cfe-commits

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)

2024-11-12 Thread kadir çetinkaya via cfe-commits


@@ -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)

2024-11-12 Thread kadir çetinkaya via cfe-commits


@@ -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)

2024-11-12 Thread kadir çetinkaya via cfe-commits


@@ -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)

2024-11-12 Thread kadir çetinkaya via cfe-commits


@@ -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)

2024-11-12 Thread kadir çetinkaya via cfe-commits

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)

2024-11-11 Thread Chuanqi Xu via cfe-commits


@@ -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)

2024-11-11 Thread Chuanqi Xu via cfe-commits

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)

2024-11-11 Thread Chuanqi Xu via cfe-commits

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)

2024-11-11 Thread Chuanqi Xu via cfe-commits


@@ -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)

2024-11-11 Thread Chuanqi Xu via cfe-commits


@@ -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)

2024-11-11 Thread Chuanqi Xu via cfe-commits


@@ -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)

2024-11-11 Thread Chuanqi Xu via cfe-commits


@@ -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)

2024-11-11 Thread Chuanqi Xu via cfe-commits


@@ -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)

2024-11-08 Thread kadir çetinkaya via cfe-commits


@@ -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)

2024-11-08 Thread kadir çetinkaya via cfe-commits


@@ -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)

2024-11-08 Thread kadir çetinkaya via cfe-commits


@@ -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)

2024-11-08 Thread kadir çetinkaya via cfe-commits

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)

2024-11-08 Thread kadir çetinkaya via cfe-commits


@@ -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)

2024-11-08 Thread kadir çetinkaya via cfe-commits


@@ -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)

2024-11-08 Thread kadir çetinkaya via cfe-commits


@@ -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)

2024-11-06 Thread Chuanqi Xu via cfe-commits

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)

2024-11-05 Thread Chuanqi Xu via cfe-commits


@@ -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)

2024-11-05 Thread Chuanqi Xu via cfe-commits


@@ -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)

2024-11-05 Thread Chuanqi Xu via cfe-commits


@@ -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)

2024-11-05 Thread Chuanqi Xu via cfe-commits


@@ -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)

2024-11-05 Thread Chuanqi Xu via cfe-commits


@@ -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)

2024-11-05 Thread Chuanqi Xu via cfe-commits


@@ -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)

2024-11-05 Thread Chuanqi Xu via cfe-commits


@@ -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)

2024-11-05 Thread Chuanqi Xu via cfe-commits

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)

2024-11-05 Thread Chuanqi Xu via cfe-commits


@@ -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)

2024-11-05 Thread Chuanqi Xu via cfe-commits


@@ -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)

2024-11-05 Thread Chuanqi Xu via cfe-commits

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)

2024-11-04 Thread kadir çetinkaya via cfe-commits


@@ -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)

2024-11-04 Thread kadir çetinkaya via cfe-commits


@@ -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)

2024-11-04 Thread kadir çetinkaya via cfe-commits


@@ -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)

2024-11-04 Thread kadir çetinkaya via cfe-commits


@@ -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)

2024-11-04 Thread kadir çetinkaya via cfe-commits


@@ -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)

2024-11-04 Thread kadir çetinkaya via cfe-commits


@@ -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)

2024-11-04 Thread kadir çetinkaya via cfe-commits


@@ -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)

2024-11-04 Thread kadir çetinkaya via cfe-commits


@@ -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)

2024-11-04 Thread kadir çetinkaya via cfe-commits


@@ -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)

2024-11-04 Thread kadir çetinkaya via cfe-commits


@@ -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)

2024-11-04 Thread kadir çetinkaya via cfe-commits

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)

2024-11-03 Thread Chuanqi Xu via cfe-commits

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)

2024-11-03 Thread Chuanqi Xu via cfe-commits


@@ -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)

2024-11-03 Thread Chuanqi Xu via cfe-commits


@@ -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)

2024-11-03 Thread Chuanqi Xu via cfe-commits


@@ -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)

2024-11-03 Thread Chuanqi Xu via cfe-commits


@@ -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)

2024-11-03 Thread Chuanqi Xu via cfe-commits


@@ -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)

2024-11-03 Thread Chuanqi Xu via cfe-commits


@@ -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)

2024-11-03 Thread Chuanqi Xu via cfe-commits

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)

2024-11-02 Thread kadir çetinkaya via cfe-commits


@@ -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)

2024-11-02 Thread kadir çetinkaya via cfe-commits


@@ -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)

2024-11-02 Thread kadir çetinkaya via cfe-commits


@@ -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)

2024-11-02 Thread kadir çetinkaya via cfe-commits


@@ -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)

2024-11-02 Thread kadir çetinkaya via cfe-commits


@@ -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)

2024-11-02 Thread kadir çetinkaya via cfe-commits

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)

2024-11-02 Thread kadir çetinkaya via cfe-commits


@@ -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)

2024-11-02 Thread kadir çetinkaya via cfe-commits

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)

2024-10-30 Thread Chuanqi Xu via cfe-commits

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)

2024-10-30 Thread Chuanqi Xu via cfe-commits


@@ -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)

2024-10-30 Thread Chuanqi Xu via cfe-commits

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)

2024-10-30 Thread Chuanqi Xu via cfe-commits


@@ -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)

2024-10-30 Thread Chuanqi Xu via cfe-commits


@@ -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)

2024-10-30 Thread Chuanqi Xu via cfe-commits

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)

2024-10-28 Thread kadir çetinkaya via cfe-commits


@@ -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)

2024-10-28 Thread kadir çetinkaya via cfe-commits


@@ -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)

2024-10-28 Thread kadir çetinkaya via cfe-commits


@@ -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)

2024-10-28 Thread Chuanqi Xu via cfe-commits


@@ -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)

2024-10-28 Thread Chuanqi Xu via cfe-commits


@@ -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)

2024-10-28 Thread Chuanqi Xu via cfe-commits


@@ -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)

2024-10-21 Thread kadir çetinkaya via cfe-commits

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)

2024-10-21 Thread kadir çetinkaya via cfe-commits


@@ -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)

2024-10-21 Thread kadir çetinkaya via cfe-commits


@@ -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)

2024-10-21 Thread kadir çetinkaya via cfe-commits


@@ -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)

2024-10-20 Thread Chuanqi Xu via cfe-commits

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)

2024-10-08 Thread Chuanqi Xu via cfe-commits

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)

2024-09-17 Thread Chuanqi Xu via cfe-commits

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)

2024-09-05 Thread Chuanqi Xu via cfe-commits

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)

2024-08-30 Thread Chuanqi Xu via cfe-commits

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

  1   2   >