llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-modules Author: Qinkun Bao (qinkunbao) <details> <summary>Changes</summary> Reverts llvm/llvm-project#<!-- -->138983 --- Full diff: https://github.com/llvm/llvm-project/pull/139987.diff 14 Files Affected: - (modified) clang-tools-extra/clangd/ModulesBuilder.cpp (+1-2) - (modified) clang/include/clang/Serialization/ModuleCache.h (+8-6) - (modified) clang/include/clang/Serialization/ModuleFile.h (+5-3) - (modified) clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h (+8-1) - (modified) clang/include/clang/Tooling/DependencyScanning/InProcessModuleCache.h (+1-1) - (modified) clang/lib/Frontend/ASTUnit.cpp (+4-6) - (modified) clang/lib/Frontend/CompilerInstance.cpp (+1-3) - (modified) clang/lib/Serialization/ASTReader.cpp (+6-3) - (modified) clang/lib/Serialization/ASTWriter.cpp (+1-1) - (modified) clang/lib/Serialization/ModuleCache.cpp (+6-14) - (modified) clang/lib/Serialization/ModuleManager.cpp (+3-3) - (modified) clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp (+4-2) - (modified) clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp (+4) - (modified) clang/lib/Tooling/DependencyScanning/InProcessModuleCache.cpp (+8-8) ``````````diff diff --git a/clang-tools-extra/clangd/ModulesBuilder.cpp b/clang-tools-extra/clangd/ModulesBuilder.cpp index 5350f1b83a7f0..c1878f91b5e16 100644 --- a/clang-tools-extra/clangd/ModulesBuilder.cpp +++ b/clang-tools-extra/clangd/ModulesBuilder.cpp @@ -207,8 +207,7 @@ bool IsModuleFileUpToDate(PathRef ModuleFilePath, Preprocessor PP(PPOpts, *Diags, LangOpts, SourceMgr, HeaderInfo, ModuleLoader); - IntrusiveRefCntPtr<ModuleCache> ModCache = - createCrossProcessModuleCache(HSOpts.BuildSessionTimestamp); + IntrusiveRefCntPtr<ModuleCache> ModCache = createCrossProcessModuleCache(); PCHContainerOperations PCHOperations; ASTReader Reader(PP, *ModCache, /*ASTContext=*/nullptr, PCHOperations.getRawReader(), {}); diff --git a/clang/include/clang/Serialization/ModuleCache.h b/clang/include/clang/Serialization/ModuleCache.h index df950906333ba..3117d954a09cc 100644 --- a/clang/include/clang/Serialization/ModuleCache.h +++ b/clang/include/clang/Serialization/ModuleCache.h @@ -33,14 +33,17 @@ class ModuleCache : public RefCountedBase<ModuleCache> { virtual std::unique_ptr<llvm::AdvisoryLock> getLock(StringRef ModuleFilename) = 0; + // TODO: Abstract away timestamps with isUpToDate() and markUpToDate(). // TODO: Consider exposing a "validation lock" API to prevent multiple clients // concurrently noticing an out-of-date module file and validating its inputs. - /// Checks whether the inputs of the module file were marked as validated. - virtual bool isMarkedUpToDate(StringRef ModuleFilename) = 0; + /// Returns the timestamp denoting the last time inputs of the module file + /// were validated. + virtual std::time_t getModuleTimestamp(StringRef ModuleFilename) = 0; - /// Marks the inputs of the module file as validated. - virtual void markUpToDate(StringRef ModuleFilename) = 0; + /// Updates the timestamp denoting the last time inputs of the module file + /// were validated. + virtual void updateModuleTimestamp(StringRef ModuleFilename) = 0; /// Returns this process's view of the module cache. virtual InMemoryModuleCache &getInMemoryModuleCache() = 0; @@ -55,8 +58,7 @@ class ModuleCache : public RefCountedBase<ModuleCache> { /// operated on by multiple processes. This instance must be used across all /// \c CompilerInstance instances participating in building modules for single /// translation unit in order to share the same \c InMemoryModuleCache. -IntrusiveRefCntPtr<ModuleCache> -createCrossProcessModuleCache(std::time_t BuildSessionTimestamp); +IntrusiveRefCntPtr<ModuleCache> createCrossProcessModuleCache(); } // namespace clang #endif diff --git a/clang/include/clang/Serialization/ModuleFile.h b/clang/include/clang/Serialization/ModuleFile.h index 5b6e31dac39ed..f20cb2f9f35ae 100644 --- a/clang/include/clang/Serialization/ModuleFile.h +++ b/clang/include/clang/Serialization/ModuleFile.h @@ -270,9 +270,11 @@ class ModuleFile { // system input files reside at [NumUserInputFiles, InputFilesLoaded.size()). unsigned NumUserInputFiles = 0; - /// Specifies whether the input files have been validated (i.e. checked - /// whether they are up-to-date). - bool InputFilesValidated = false; + /// If non-zero, specifies the time when we last validated input + /// files. Zero means we never validated them. + /// + /// The time is specified in seconds since the start of the Epoch. + uint64_t InputFilesValidationTimestamp = 0; // === Source Locations === diff --git a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h index 22f670cb821e2..4e97c7bc9f36e 100644 --- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h +++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h @@ -12,6 +12,7 @@ #include "clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h" #include "clang/Tooling/DependencyScanning/InProcessModuleCache.h" #include "llvm/ADT/BitmaskEnum.h" +#include "llvm/Support/Chrono.h" namespace clang { namespace tooling { @@ -84,7 +85,9 @@ class DependencyScanningService { DependencyScanningService( ScanningMode Mode, ScanningOutputFormat Format, ScanningOptimizations OptimizeArgs = ScanningOptimizations::Default, - bool EagerLoadModules = false, bool TraceVFS = false); + bool EagerLoadModules = false, bool TraceVFS = false, + std::time_t BuildSessionTimestamp = + llvm::sys::toTimeT(std::chrono::system_clock::now())); ScanningMode getMode() const { return Mode; } @@ -102,6 +105,8 @@ class DependencyScanningService { ModuleCacheEntries &getModuleCacheEntries() { return ModCacheEntries; } + std::time_t getBuildSessionTimestamp() const { return BuildSessionTimestamp; } + private: const ScanningMode Mode; const ScanningOutputFormat Format; @@ -115,6 +120,8 @@ class DependencyScanningService { DependencyScanningFilesystemSharedCache SharedCache; /// The global module cache entries. ModuleCacheEntries ModCacheEntries; + /// The build session timestamp. + std::time_t BuildSessionTimestamp; }; } // end namespace dependencies diff --git a/clang/include/clang/Tooling/DependencyScanning/InProcessModuleCache.h b/clang/include/clang/Tooling/DependencyScanning/InProcessModuleCache.h index fa11af2be5a67..213e60b39c199 100644 --- a/clang/include/clang/Tooling/DependencyScanning/InProcessModuleCache.h +++ b/clang/include/clang/Tooling/DependencyScanning/InProcessModuleCache.h @@ -20,7 +20,7 @@ namespace tooling { namespace dependencies { struct ModuleCacheEntry { std::shared_mutex CompilationMutex; - std::atomic<bool> UpToDate = false; + std::atomic<std::time_t> Timestamp = 0; }; struct ModuleCacheEntries { diff --git a/clang/lib/Frontend/ASTUnit.cpp b/clang/lib/Frontend/ASTUnit.cpp index eb8483dcfc008..5a79fe070c384 100644 --- a/clang/lib/Frontend/ASTUnit.cpp +++ b/clang/lib/Frontend/ASTUnit.cpp @@ -829,10 +829,9 @@ std::unique_ptr<ASTUnit> ASTUnit::LoadFromASTFile( AST->SourceMgr = new SourceManager(AST->getDiagnostics(), AST->getFileManager(), UserFilesAreVolatile); + AST->ModCache = createCrossProcessModuleCache(); AST->HSOpts = std::make_unique<HeaderSearchOptions>(HSOpts); AST->HSOpts->ModuleFormat = std::string(PCHContainerRdr.getFormats().front()); - AST->ModCache = - createCrossProcessModuleCache(AST->HSOpts->BuildSessionTimestamp); AST->HeaderInfo.reset(new HeaderSearch(AST->getHeaderSearchOpts(), AST->getSourceManager(), AST->getDiagnostics(), @@ -1549,8 +1548,7 @@ ASTUnit::create(std::shared_ptr<CompilerInvocation> CI, AST->UserFilesAreVolatile = UserFilesAreVolatile; AST->SourceMgr = new SourceManager(AST->getDiagnostics(), *AST->FileMgr, UserFilesAreVolatile); - AST->ModCache = createCrossProcessModuleCache( - AST->Invocation->getHeaderSearchOpts().BuildSessionTimestamp); + AST->ModCache = createCrossProcessModuleCache(); return AST; } @@ -1836,6 +1834,7 @@ std::unique_ptr<ASTUnit> ASTUnit::LoadFromCommandLine( AST->FileMgr = new FileManager(AST->FileSystemOpts, VFS); AST->StorePreamblesInMemory = StorePreamblesInMemory; AST->PreambleStoragePath = PreambleStoragePath; + AST->ModCache = createCrossProcessModuleCache(); AST->OnlyLocalDecls = OnlyLocalDecls; AST->CaptureDiagnostics = CaptureDiagnostics; AST->TUKind = TUKind; @@ -1844,8 +1843,6 @@ std::unique_ptr<ASTUnit> ASTUnit::LoadFromCommandLine( = IncludeBriefCommentsInCodeCompletion; AST->UserFilesAreVolatile = UserFilesAreVolatile; AST->Invocation = CI; - AST->ModCache = createCrossProcessModuleCache( - AST->Invocation->getHeaderSearchOpts().BuildSessionTimestamp); AST->SkipFunctionBodies = SkipFunctionBodies; if (ForSerialization) AST->WriterData.reset(new ASTWriterData(*AST->ModCache)); @@ -2381,6 +2378,7 @@ bool ASTUnit::serialize(raw_ostream &OS) { SmallString<128> Buffer; llvm::BitstreamWriter Stream(Buffer); + IntrusiveRefCntPtr<ModuleCache> ModCache = createCrossProcessModuleCache(); ASTWriter Writer(Stream, Buffer, *ModCache, {}); return serializeUnit(Writer, Buffer, getSema(), OS); } diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp index b3a23e64dbada..503d36467653e 100644 --- a/clang/lib/Frontend/CompilerInstance.cpp +++ b/clang/lib/Frontend/CompilerInstance.cpp @@ -72,9 +72,7 @@ CompilerInstance::CompilerInstance( ModuleCache *ModCache) : ModuleLoader(/*BuildingModule=*/ModCache), Invocation(std::move(Invocation)), - ModCache(ModCache ? ModCache - : createCrossProcessModuleCache( - getHeaderSearchOpts().BuildSessionTimestamp)), + ModCache(ModCache ? ModCache : createCrossProcessModuleCache()), ThePCHContainerOperations(std::move(PCHContainerOps)) { assert(this->Invocation && "Invocation must not be null"); } diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp index 2462b5c1f1421..d068f5e163176 100644 --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -3103,7 +3103,8 @@ ASTReader::ReadControlBlock(ModuleFile &F, unsigned N = ValidateSystemInputs ? NumInputs : NumUserInputs; if (HSOpts.ModulesValidateOncePerBuildSession && - F.InputFilesValidated && F.Kind == MK_ImplicitModule) + F.InputFilesValidationTimestamp > HSOpts.BuildSessionTimestamp && + F.Kind == MK_ImplicitModule) N = ForceValidateUserInputs ? NumUserInputs : 0; for (unsigned I = 0; I < N; ++I) { @@ -4949,8 +4950,10 @@ ASTReader::ASTReadResult ASTReader::ReadAST(StringRef FileName, ModuleKind Type, // timestamp files are up-to-date in this build session. for (unsigned I = 0, N = Loaded.size(); I != N; ++I) { ImportedModule &M = Loaded[I]; - if (M.Mod->Kind == MK_ImplicitModule && !M.Mod->InputFilesValidated) - getModuleManager().getModuleCache().markUpToDate(M.Mod->FileName); + if (M.Mod->Kind == MK_ImplicitModule && + M.Mod->InputFilesValidationTimestamp < HSOpts.BuildSessionTimestamp) + getModuleManager().getModuleCache().updateModuleTimestamp( + M.Mod->FileName); } } diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp index 491149b33f1d8..1b3d3c22aa9f5 100644 --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -5391,7 +5391,7 @@ ASTWriter::WriteAST(llvm::PointerUnion<Sema *, Preprocessor *> Subject, if (WritingModule && PPRef.getHeaderSearchInfo() .getHeaderSearchOpts() .ModulesValidateOncePerBuildSession) - ModCache.markUpToDate(OutputFile); + ModCache.updateModuleTimestamp(OutputFile); if (ShouldCacheASTInMemory) { // Construct MemoryBuffer and update buffer manager. diff --git a/clang/lib/Serialization/ModuleCache.cpp b/clang/lib/Serialization/ModuleCache.cpp index 43f8ff8d681fe..4ae49c4ec9a05 100644 --- a/clang/lib/Serialization/ModuleCache.cpp +++ b/clang/lib/Serialization/ModuleCache.cpp @@ -20,12 +20,7 @@ namespace { class CrossProcessModuleCache : public ModuleCache { InMemoryModuleCache InMemory; - std::time_t BuildSessionTimestamp; - public: - explicit CrossProcessModuleCache(std::time_t BuildSessionTimestamp) - : BuildSessionTimestamp(BuildSessionTimestamp) {} - void prepareForGetLock(StringRef ModuleFilename) override { // FIXME: Do this in LockFileManager and only if the directory doesn't // exist. @@ -38,17 +33,16 @@ class CrossProcessModuleCache : public ModuleCache { return std::make_unique<llvm::LockFileManager>(ModuleFilename); } - bool isMarkedUpToDate(StringRef ModuleFilename) override { + std::time_t getModuleTimestamp(StringRef ModuleFilename) override { std::string TimestampFilename = serialization::ModuleFile::getTimestampFilename(ModuleFilename); llvm::sys::fs::file_status Status; if (llvm::sys::fs::status(ModuleFilename, Status) != std::error_code{}) - return false; - return llvm::sys::toTimeT(Status.getLastModificationTime()) > - BuildSessionTimestamp; + return 0; + return llvm::sys::toTimeT(Status.getLastModificationTime()); } - void markUpToDate(StringRef ModuleFilename) override { + void updateModuleTimestamp(StringRef ModuleFilename) override { // Overwrite the timestamp file contents so that file's mtime changes. std::error_code EC; llvm::raw_fd_ostream OS( @@ -68,8 +62,6 @@ class CrossProcessModuleCache : public ModuleCache { }; } // namespace -IntrusiveRefCntPtr<ModuleCache> -clang::createCrossProcessModuleCache(std::time_t BuildSessionTimestamp) { - return llvm::makeIntrusiveRefCnt<CrossProcessModuleCache>( - BuildSessionTimestamp); +IntrusiveRefCntPtr<ModuleCache> clang::createCrossProcessModuleCache() { + return llvm::makeIntrusiveRefCnt<CrossProcessModuleCache>(); } diff --git a/clang/lib/Serialization/ModuleManager.cpp b/clang/lib/Serialization/ModuleManager.cpp index 9fd7505726973..fa9533b7efd78 100644 --- a/clang/lib/Serialization/ModuleManager.cpp +++ b/clang/lib/Serialization/ModuleManager.cpp @@ -174,11 +174,11 @@ ModuleManager::addModule(StringRef FileName, ModuleKind Type, NewModule->Index = Chain.size(); NewModule->FileName = FileName.str(); NewModule->ImportLoc = ImportLoc; - NewModule->InputFilesValidated = false; + NewModule->InputFilesValidationTimestamp = 0; if (NewModule->Kind == MK_ImplicitModule) - NewModule->InputFilesValidated = - ModCache->isMarkedUpToDate(NewModule->FileName); + NewModule->InputFilesValidationTimestamp = + ModCache->getModuleTimestamp(NewModule->FileName); // Load the contents of the module if (std::unique_ptr<llvm::MemoryBuffer> Buffer = lookupBuffer(FileName)) { diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp index 96fe40c079c65..7f40c99f07287 100644 --- a/clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp +++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp @@ -14,6 +14,8 @@ using namespace dependencies; DependencyScanningService::DependencyScanningService( ScanningMode Mode, ScanningOutputFormat Format, - ScanningOptimizations OptimizeArgs, bool EagerLoadModules, bool TraceVFS) + ScanningOptimizations OptimizeArgs, bool EagerLoadModules, bool TraceVFS, + std::time_t BuildSessionTimestamp) : Mode(Mode), Format(Format), OptimizeArgs(OptimizeArgs), - EagerLoadModules(EagerLoadModules), TraceVFS(TraceVFS) {} + EagerLoadModules(EagerLoadModules), TraceVFS(TraceVFS), + BuildSessionTimestamp(BuildSessionTimestamp) {} diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp index e30e997e40581..21eea72b198b3 100644 --- a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp +++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp @@ -428,6 +428,10 @@ class DependencyScanningAction : public tooling::ToolAction { ScanInstance.getPreprocessorOpts().AllowPCHWithDifferentModulesCachePath = true; + if (ScanInstance.getHeaderSearchOpts().ModulesValidateOncePerBuildSession) + ScanInstance.getHeaderSearchOpts().BuildSessionTimestamp = + Service.getBuildSessionTimestamp(); + ScanInstance.getFrontendOpts().GenerateGlobalModuleIndex = false; ScanInstance.getFrontendOpts().UseGlobalModuleIndex = false; // This will prevent us compiling individual modules asynchronously since diff --git a/clang/lib/Tooling/DependencyScanning/InProcessModuleCache.cpp b/clang/lib/Tooling/DependencyScanning/InProcessModuleCache.cpp index ccfe42c092f58..80db2d47d940e 100644 --- a/clang/lib/Tooling/DependencyScanning/InProcessModuleCache.cpp +++ b/clang/lib/Tooling/DependencyScanning/InProcessModuleCache.cpp @@ -75,29 +75,29 @@ class InProcessModuleCache : public ModuleCache { return std::make_unique<ReaderWriterLock>(CompilationMutex); } - bool isMarkedUpToDate(StringRef Filename) override { - auto &IsUpToDate = [&]() -> std::atomic<bool> & { + std::time_t getModuleTimestamp(StringRef Filename) override { + auto &Timestamp = [&]() -> std::atomic<std::time_t> & { std::lock_guard<std::mutex> Lock(Entries.Mutex); auto &Entry = Entries.Map[Filename]; if (!Entry) Entry = std::make_unique<ModuleCacheEntry>(); - return Entry->UpToDate; + return Entry->Timestamp; }(); - return IsUpToDate; + return Timestamp.load(); } - void markUpToDate(StringRef Filename) override { + void updateModuleTimestamp(StringRef Filename) override { // Note: This essentially replaces FS contention with mutex contention. - auto &IsUpToDate = [&]() -> std::atomic<bool> & { + auto &Timestamp = [&]() -> std::atomic<std::time_t> & { std::lock_guard<std::mutex> Lock(Entries.Mutex); auto &Entry = Entries.Map[Filename]; if (!Entry) Entry = std::make_unique<ModuleCacheEntry>(); - return Entry->UpToDate; + return Entry->Timestamp; }(); - IsUpToDate = true; + Timestamp.store(llvm::sys::toTimeT(std::chrono::system_clock::now())); } InMemoryModuleCache &getInMemoryModuleCache() override { return InMemory; } `````````` </details> https://github.com/llvm/llvm-project/pull/139987 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
