https://github.com/jansvoboda11 created https://github.com/llvm/llvm-project/pull/183787
Implicitly-built modules are stored in the in-memory cache of the `CompilerInstance` responsible for building it. This means it's safe to release the lock right after building it, and read it outside of the critical section from the in-memory module cache. This speeds up dependency scanning in a statistically significant way, somewhere between 0.5% and 1.0%. >From dc57d0174ae326bd85ee98d9c192454880e79a2c Mon Sep 17 00:00:00 2001 From: Jan Svoboda <[email protected]> Date: Fri, 27 Feb 2026 10:05:10 -0800 Subject: [PATCH] [clang][modules] Unlock before reading just-built PCM from the in-memory module cache --- clang/lib/Frontend/CompilerInstance.cpp | 92 ++++++++++++++++--------- 1 file changed, 58 insertions(+), 34 deletions(-) diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp index 1e256a9d5614c..b9ef2447b7b2b 100644 --- a/clang/lib/Frontend/CompilerInstance.cpp +++ b/clang/lib/Frontend/CompilerInstance.cpp @@ -1402,6 +1402,7 @@ std::unique_ptr<CompilerInstance> CompilerInstance::cloneForModuleCompile( } /// Read the AST right after compiling the module. +/// Returns true on success, false on failure. static bool readASTAfterCompileModule(CompilerInstance &ImportingInstance, SourceLocation ImportLoc, SourceLocation ModuleNameLoc, @@ -1441,13 +1442,12 @@ static bool readASTAfterCompileModule(CompilerInstance &ImportingInstance, return false; } -/// Compile a module in a separate compiler instance and read the AST, -/// returning true if the module compiles without errors. -static bool compileModuleAndReadASTImpl(CompilerInstance &ImportingInstance, - SourceLocation ImportLoc, - SourceLocation ModuleNameLoc, - Module *Module, - StringRef ModuleFileName) { +/// Compile a module in a separate compiler instance. +/// Returns true on success, false on failure. +static bool compileModuleImpl(CompilerInstance &ImportingInstance, + SourceLocation ImportLoc, + SourceLocation ModuleNameLoc, Module *Module, + StringRef ModuleFileName) { { auto Instance = ImportingInstance.cloneForModuleCompile( ModuleNameLoc, Module, ModuleFileName); @@ -1470,20 +1470,25 @@ static bool compileModuleAndReadASTImpl(CompilerInstance &ImportingInstance, ImportingInstance.getModuleCache().updateModuleTimestamp(ModuleFileName); } - return readASTAfterCompileModule(ImportingInstance, ImportLoc, ModuleNameLoc, - Module, ModuleFileName, - /*OutOfDate=*/nullptr, /*Missing=*/nullptr); + return true; } -/// Compile a module in a separate compiler instance and read the AST, -/// returning true if the module compiles without errors, using a lock manager -/// to avoid building the same module in multiple compiler instances. -/// -/// Uses a lock file manager and exponential backoff to reduce the chances that -/// multiple instances will compete to create the same module. On timeout, -/// deletes the lock file in order to avoid deadlock from crashing processes or -/// bugs in the lock file manager. -static bool compileModuleAndReadASTBehindLock( +/// The result of `compileModuleBehindLockOrRead()`. +enum class CompileOrReadResult : uint8_t { + /// We failed to compile the module. + FailedToCompile, + /// We successfully compiled the module and we still need to read it. + Compiled, + /// We failed to read the module file compiled by another instance. + FailedToRead, + /// We read a module file compiled by another instance. + Read, +}; + +/// Attempt to compile the module in a separate compiler instance behind a lock +/// (to avoid building the same module in multiple compiler instances), or read +/// the AST produced by another compiler instance. +static CompileOrReadResult compileModuleBehindLockOrRead( CompilerInstance &ImportingInstance, SourceLocation ImportLoc, SourceLocation ModuleNameLoc, Module *Module, StringRef ModuleFileName) { DiagnosticsEngine &Diags = ImportingInstance.getDiagnostics(); @@ -1503,13 +1508,17 @@ static bool compileModuleAndReadASTBehindLock( // related errors. Diags.Report(ModuleNameLoc, diag::remark_module_lock_failure) << Module->Name << toString(std::move(Err)); - return compileModuleAndReadASTImpl(ImportingInstance, ImportLoc, - ModuleNameLoc, Module, ModuleFileName); + if (!compileModuleImpl(ImportingInstance, ImportLoc, ModuleNameLoc, + Module, ModuleFileName)) + return CompileOrReadResult::FailedToCompile; + return CompileOrReadResult::Compiled; } if (Owned) { // We're responsible for building the module ourselves. - return compileModuleAndReadASTImpl(ImportingInstance, ImportLoc, - ModuleNameLoc, Module, ModuleFileName); + if (!compileModuleImpl(ImportingInstance, ImportLoc, ModuleNameLoc, + Module, ModuleFileName)) + return CompileOrReadResult::FailedToCompile; + return CompileOrReadResult::Compiled; } // Someone else is responsible for building the module. Wait for them to @@ -1537,9 +1546,9 @@ static bool compileModuleAndReadASTBehindLock( bool Missing = false; if (readASTAfterCompileModule(ImportingInstance, ImportLoc, ModuleNameLoc, Module, ModuleFileName, &OutOfDate, &Missing)) - return true; + return CompileOrReadResult::Read; if (!OutOfDate && !Missing) - return false; + return CompileOrReadResult::FailedToRead; // The module may be missing or out of date in the presence of file system // races. It may also be out of date if one of its imports depends on header @@ -1556,15 +1565,30 @@ static bool compileModuleAndReadAST(CompilerInstance &ImportingInstance, SourceLocation ImportLoc, SourceLocation ModuleNameLoc, Module *Module, StringRef ModuleFileName) { - return ImportingInstance.getInvocation() - .getFrontendOpts() - .BuildingImplicitModuleUsesLock - ? compileModuleAndReadASTBehindLock(ImportingInstance, ImportLoc, - ModuleNameLoc, Module, - ModuleFileName) - : compileModuleAndReadASTImpl(ImportingInstance, ImportLoc, - ModuleNameLoc, Module, - ModuleFileName); + if (ImportingInstance.getInvocation() + .getFrontendOpts() + .BuildingImplicitModuleUsesLock) { + switch (compileModuleBehindLockOrRead( + ImportingInstance, ImportLoc, ModuleNameLoc, Module, ModuleFileName)) { + case CompileOrReadResult::FailedToRead: + case CompileOrReadResult::FailedToCompile: + return false; + case CompileOrReadResult::Read: + return true; + case CompileOrReadResult::Compiled: + // We successfully compiled the module under a lock. Let's read it from + // the in-memory module cache now. + break; + } + } else { + if (!compileModuleImpl(ImportingInstance, ImportLoc, ModuleNameLoc, Module, + ModuleFileName)) + return false; + } + + return readASTAfterCompileModule(ImportingInstance, ImportLoc, ModuleNameLoc, + Module, ModuleFileName, + /*OutOfDate=*/nullptr, /*Missing=*/nullptr); } /// Diagnose differences between the current definition of the given _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
