From 677e15ffee2ecc9c1c8f46fd77cab4b5afb59640 Mon Sep 17 00:00:00 2001 From: Douglas Gregor Date: Tue, 19 Mar 2013 00:28:20 +0000 Subject: Eliminate race condition between module rebuild and the global module index. The global module index was querying the file manager for each of the module files it knows about at load time, to prune out any out-of-date information. The file manager would then cache the results of the stat() falls used to find that module file. Later, the same translation unit could end up trying to import one of the module files that had previously been ignored by the module cache, but after some other Clang instance rebuilt the module file to bring it up-to-date. The stale stat() results in the file manager would trigger a second rebuild of the already-up-to-date module, causing failures down the line. The global module index now lazily resolves its module file references to actual AST reader module files only after the module file has been loaded, eliminating the stat-caching race. Moreover, the AST reader can communicate to its caller that a module file is missing (rather than simply being out-of-date), allowing us to simplify the module-loading logic and allowing the compiler to recover if a dependent module file ends up getting deleted. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@177367 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Serialization/ModuleManager.cpp | 116 ++++++++++++++++++++++++++++-------- 1 file changed, 90 insertions(+), 26 deletions(-) (limited to 'lib/Serialization/ModuleManager.cpp') diff --git a/lib/Serialization/ModuleManager.cpp b/lib/Serialization/ModuleManager.cpp index b9f4d888f3..a9f4794e10 100644 --- a/lib/Serialization/ModuleManager.cpp +++ b/lib/Serialization/ModuleManager.cpp @@ -11,9 +11,11 @@ // modules for the ASTReader. // //===----------------------------------------------------------------------===// +#include "clang/Lex/ModuleMap.h" #include "clang/Serialization/ModuleManager.h" #include "clang/Serialization/GlobalModuleIndex.h" #include "llvm/Support/MemoryBuffer.h" +#include "llvm/Support/PathV2.h" #include "llvm/Support/raw_ostream.h" #include "llvm/Support/system_error.h" @@ -36,18 +38,28 @@ llvm::MemoryBuffer *ModuleManager::lookupBuffer(StringRef Name) { return InMemoryBuffers[Entry]; } -std::pair +ModuleManager::AddModuleResult ModuleManager::addModule(StringRef FileName, ModuleKind Type, SourceLocation ImportLoc, ModuleFile *ImportedBy, - unsigned Generation, std::string &ErrorStr) { - const FileEntry *Entry = FileMgr.getFile(FileName, /*openFile=*/false, - /*cacheFailure=*/false); + unsigned Generation, + off_t ExpectedSize, time_t ExpectedModTime, + ModuleFile *&Module, + std::string &ErrorStr) { + Module = 0; + + // Look for the file entry. This only fails if the expected size or + // modification time differ. + const FileEntry *Entry; + if (lookupModuleFile(FileName, ExpectedSize, ExpectedModTime, Entry)) + return OutOfDate; + if (!Entry && FileName != "-") { ErrorStr = "file not found"; - return std::make_pair(static_cast(0), false); + return Missing; } - - // Check whether we already loaded this module, before + + // Check whether we already loaded this module, before + AddModuleResult Result = AlreadyLoaded; ModuleFile *&ModuleEntry = Modules[Entry]; bool NewModule = false; if (!ModuleEntry) { @@ -77,12 +89,15 @@ ModuleManager::addModule(StringRef FileName, ModuleKind Type, New->Buffer.reset(FileMgr.getBufferForFile(FileName, &ErrorStr)); if (!New->Buffer) - return std::make_pair(static_cast(0), false); + return Missing; } // Initialize the stream New->StreamFile.init((const unsigned char *)New->Buffer->getBufferStart(), - (const unsigned char *)New->Buffer->getBufferEnd()); } + (const unsigned char *)New->Buffer->getBufferEnd()); + + Result = NewlyLoaded; + } if (ImportedBy) { ModuleEntry->ImportedBy.insert(ImportedBy); @@ -93,8 +108,9 @@ ModuleManager::addModule(StringRef FileName, ModuleKind Type, ModuleEntry->DirectlyImported = true; } - - return std::make_pair(ModuleEntry, NewModule); + + Module = ModuleEntry; + return NewModule? NewlyLoaded : AlreadyLoaded; } namespace { @@ -113,7 +129,8 @@ namespace { }; } -void ModuleManager::removeModules(ModuleIterator first, ModuleIterator last) { +void ModuleManager::removeModules(ModuleIterator first, ModuleIterator last, + ModuleMap *modMap) { if (first == last) return; @@ -129,6 +146,14 @@ void ModuleManager::removeModules(ModuleIterator first, ModuleIterator last) { // Delete the modules and erase them from the various structures. for (ModuleIterator victim = first; victim != last; ++victim) { Modules.erase((*victim)->File); + + FileMgr.invalidateCache((*victim)->File); + if (modMap) { + StringRef ModuleName = llvm::sys::path::stem((*victim)->FileName); + if (Module *mod = modMap->findModule(ModuleName)) { + mod->setASTFile(0); + } + } delete *victim; } @@ -151,18 +176,8 @@ void ModuleManager::updateModulesInCommonWithGlobalIndex() { return; // Collect the set of modules known to the global index. - SmallVector KnownModules; - GlobalIndex->getKnownModules(KnownModules); - - // Map those modules to AST files known to the module manager. - for (unsigned I = 0, N = KnownModules.size(); I != N; ++I) { - llvm::DenseMap::iterator Known - = Modules.find(KnownModules[I]); - if (Known == Modules.end()) - continue; - - ModulesInCommonWithGlobalIndex.push_back(Known->second); - } + GlobalIndex->noteAdditionalModulesLoaded(); + GlobalIndex->getKnownModules(ModulesInCommonWithGlobalIndex); } ModuleManager::VisitState *ModuleManager::allocateVisitState() { @@ -186,6 +201,9 @@ void ModuleManager::returnVisitState(VisitState *State) { void ModuleManager::setGlobalIndex(GlobalModuleIndex *Index) { GlobalIndex = Index; + if (GlobalIndex) { + GlobalIndex->setResolver(this); + } updateModulesInCommonWithGlobalIndex(); } @@ -201,7 +219,7 @@ ModuleManager::~ModuleManager() { void ModuleManager::visit(bool (*Visitor)(ModuleFile &M, void *UserData), void *UserData, - llvm::SmallPtrSet *ModuleFilesHit) { + llvm::SmallPtrSet *ModuleFilesHit) { // If the visitation order vector is the wrong size, recompute the order. if (VisitOrder.size() != Chain.size()) { unsigned N = size(); @@ -268,7 +286,7 @@ ModuleManager::visit(bool (*Visitor)(ModuleFile &M, void *UserData), for (unsigned I = 0, N = ModulesInCommonWithGlobalIndex.size(); I != N; ++I) { ModuleFile *M = ModulesInCommonWithGlobalIndex[I]; - if (!ModuleFilesHit->count(M->File)) + if (!ModuleFilesHit->count(M)) State->VisitNumber[M->Index] = VisitNumber; } } @@ -354,6 +372,52 @@ void ModuleManager::visitDepthFirst(bool (*Visitor)(ModuleFile &M, bool Preorder } } +bool ModuleManager::lookupModuleFile(StringRef FileName, + off_t ExpectedSize, + time_t ExpectedModTime, + const FileEntry *&File) { + File = FileMgr.getFile(FileName, /*openFile=*/false, /*cacheFailure=*/false); + + if (!File && FileName != "-") { + return false; + } + + if ((ExpectedSize && ExpectedSize != File->getSize()) || + (ExpectedModTime && ExpectedModTime != File->getModificationTime())) { + return true; + } + + return false; +} + +bool ModuleManager::resolveModuleFileName(StringRef FileName, + off_t ExpectedSize, + time_t ExpectedModTime, + ModuleFile *&File) { + File = 0; + + // Look for the file entry corresponding to this name. + const FileEntry *F; + if (lookupModuleFile(FileName, ExpectedSize, ExpectedModTime, F)) + return true; + + // If there is no file, we've succeeded (trivially). + if (!F) + return false; + + // Determine whether we have a module file associated with this file entry. + llvm::DenseMap::iterator Known + = Modules.find(F); + if (Known == Modules.end()) { + // We don't know about this module file; invalidate the cache. + FileMgr.invalidateCache(F); + return false; + } + + File = Known->second; + return false; +} + #ifndef NDEBUG namespace llvm { template<> -- cgit v1.2.3-18-g5258 From 0beab2746464ff7e871b2ec9c9b7fd9e8bc53494 Mon Sep 17 00:00:00 2001 From: Benjamin Kramer Date: Thu, 21 Mar 2013 19:47:38 +0000 Subject: Remove unused variable. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@177657 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Serialization/ModuleManager.cpp | 3 --- 1 file changed, 3 deletions(-) (limited to 'lib/Serialization/ModuleManager.cpp') diff --git a/lib/Serialization/ModuleManager.cpp b/lib/Serialization/ModuleManager.cpp index a9f4794e10..7384da5404 100644 --- a/lib/Serialization/ModuleManager.cpp +++ b/lib/Serialization/ModuleManager.cpp @@ -59,7 +59,6 @@ ModuleManager::addModule(StringRef FileName, ModuleKind Type, } // Check whether we already loaded this module, before - AddModuleResult Result = AlreadyLoaded; ModuleFile *&ModuleEntry = Modules[Entry]; bool NewModule = false; if (!ModuleEntry) { @@ -95,8 +94,6 @@ ModuleManager::addModule(StringRef FileName, ModuleKind Type, // Initialize the stream New->StreamFile.init((const unsigned char *)New->Buffer->getBufferStart(), (const unsigned char *)New->Buffer->getBufferEnd()); - - Result = NewlyLoaded; } if (ImportedBy) { -- cgit v1.2.3-18-g5258 From fa69fc19121da3fc5673ccc00d4e8afa5b540a4f Mon Sep 17 00:00:00 2001 From: Douglas Gregor Date: Fri, 22 Mar 2013 18:50:14 +0000 Subject: Simplify ModuleManager/GlobalModuleIndex interaction to eliminate a pile of extraneous stats(). The refactoring in r177367 introduced a serious performance bug where the "lazy" resolution of module file names in the global module index to actual module file entries in the module manager would perform repeated negative stats(). The new interaction requires the module manager to inform the global module index when a module file has been loaded, eliminating the extraneous stat()s and a bunch of bookkeeping on both sides. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@177750 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Serialization/ModuleManager.cpp | 65 ++++++++++--------------------------- 1 file changed, 18 insertions(+), 47 deletions(-) (limited to 'lib/Serialization/ModuleManager.cpp') diff --git a/lib/Serialization/ModuleManager.cpp b/lib/Serialization/ModuleManager.cpp index 7384da5404..193a38b973 100644 --- a/lib/Serialization/ModuleManager.cpp +++ b/lib/Serialization/ModuleManager.cpp @@ -166,17 +166,6 @@ void ModuleManager::addInMemoryBuffer(StringRef FileName, InMemoryBuffers[Entry] = Buffer; } -void ModuleManager::updateModulesInCommonWithGlobalIndex() { - ModulesInCommonWithGlobalIndex.clear(); - - if (!GlobalIndex) - return; - - // Collect the set of modules known to the global index. - GlobalIndex->noteAdditionalModulesLoaded(); - GlobalIndex->getKnownModules(ModulesInCommonWithGlobalIndex); -} - ModuleManager::VisitState *ModuleManager::allocateVisitState() { // Fast path: if we have a cached state, use it. if (FirstVisitState) { @@ -198,10 +187,25 @@ void ModuleManager::returnVisitState(VisitState *State) { void ModuleManager::setGlobalIndex(GlobalModuleIndex *Index) { GlobalIndex = Index; - if (GlobalIndex) { - GlobalIndex->setResolver(this); + if (!GlobalIndex) { + ModulesInCommonWithGlobalIndex.clear(); + return; } - updateModulesInCommonWithGlobalIndex(); + + // Notify the global module index about all of the modules we've already + // loaded. + for (unsigned I = 0, N = Chain.size(); I != N; ++I) { + if (!GlobalIndex->loadedModuleFile(Chain[I])) { + ModulesInCommonWithGlobalIndex.push_back(Chain[I]); + } + } +} + +void ModuleManager::moduleFileAccepted(ModuleFile *MF) { + if (!GlobalIndex || GlobalIndex->loadedModuleFile(MF)) + return; + + ModulesInCommonWithGlobalIndex.push_back(MF); } ModuleManager::ModuleManager(FileManager &FileMgr) @@ -264,11 +268,6 @@ ModuleManager::visit(bool (*Visitor)(ModuleFile &M, void *UserData), assert(VisitOrder.size() == N && "Visitation order is wrong?"); - // We may need to update the set of modules we have in common with the - // global module index, since modules could have been added to the module - // manager since we loaded the global module index. - updateModulesInCommonWithGlobalIndex(); - delete FirstVisitState; FirstVisitState = 0; } @@ -387,34 +386,6 @@ bool ModuleManager::lookupModuleFile(StringRef FileName, return false; } -bool ModuleManager::resolveModuleFileName(StringRef FileName, - off_t ExpectedSize, - time_t ExpectedModTime, - ModuleFile *&File) { - File = 0; - - // Look for the file entry corresponding to this name. - const FileEntry *F; - if (lookupModuleFile(FileName, ExpectedSize, ExpectedModTime, F)) - return true; - - // If there is no file, we've succeeded (trivially). - if (!F) - return false; - - // Determine whether we have a module file associated with this file entry. - llvm::DenseMap::iterator Known - = Modules.find(F); - if (Known == Modules.end()) { - // We don't know about this module file; invalidate the cache. - FileMgr.invalidateCache(F); - return false; - } - - File = Known->second; - return false; -} - #ifndef NDEBUG namespace llvm { template<> -- cgit v1.2.3-18-g5258 From c544ba09695e300f31355af342258bd57619e737 Mon Sep 17 00:00:00 2001 From: Douglas Gregor Date: Wed, 27 Mar 2013 16:47:18 +0000 Subject: Introduce -module-file-info option that provides information about a particular module file. This option can be useful for end users who want to know why they ended up with a ton of different variants of the "std" module in their module cache. This problem should go away over time, as we reduce the need for module variants, but it will never go away entirely. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@178148 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Serialization/ModuleManager.cpp | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) (limited to 'lib/Serialization/ModuleManager.cpp') diff --git a/lib/Serialization/ModuleManager.cpp b/lib/Serialization/ModuleManager.cpp index 193a38b973..f3d53adafa 100644 --- a/lib/Serialization/ModuleManager.cpp +++ b/lib/Serialization/ModuleManager.cpp @@ -29,7 +29,19 @@ using namespace serialization; ModuleFile *ModuleManager::lookup(StringRef Name) { const FileEntry *Entry = FileMgr.getFile(Name, /*openFile=*/false, /*cacheFailure=*/false); - return Modules[Entry]; + if (Entry) + return lookup(Entry); + + return 0; +} + +ModuleFile *ModuleManager::lookup(const FileEntry *File) { + llvm::DenseMap::iterator Known + = Modules.find(File); + if (Known == Modules.end()) + return 0; + + return Known->second; } llvm::MemoryBuffer *ModuleManager::lookupBuffer(StringRef Name) { -- cgit v1.2.3-18-g5258