r209922 - Invalidate the file system cache entries for files that may rebuild

Ben Langmuir blangmuir at apple.com
Fri May 30 14:20:54 PDT 2014


Author: benlangmuir
Date: Fri May 30 16:20:54 2014
New Revision: 209922

URL: http://llvm.org/viewvc/llvm-project?rev=209922&view=rev
Log:
Invalidate the file system cache entries for files that may rebuild

This reapplies r209910 with a fix for the assertion failures hit on the
buildbots.

original commit message:
I thought we could get away without this, but it means that the
FileEntry objects actually refer to the wrong files, since pcms are not
updated inplace, they are atomically renamed into place after compiling
a module.

So we are close to the original behaviour of invalidating the cache for
all modules being removed, but now we should only invalidate the ones
that depend on whichever module failed to load.

Unfortunately I haven't come up with a new test that didn't require
a race between parallel invocations of clang.

<rdar://problem/17038180>

Modified:
    cfe/trunk/lib/Serialization/ModuleManager.cpp

Modified: cfe/trunk/lib/Serialization/ModuleManager.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ModuleManager.cpp?rev=209922&r1=209921&r2=209922&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ModuleManager.cpp (original)
+++ cfe/trunk/lib/Serialization/ModuleManager.cpp Fri May 30 16:20:54 2014
@@ -135,19 +135,31 @@ ModuleManager::addModule(StringRef FileN
   return NewModule? NewlyLoaded : AlreadyLoaded;
 }
 
+static void getModuleFileAncestors(
+    ModuleFile *F,
+    llvm::SmallPtrSetImpl<ModuleFile *> &Ancestors) {
+  Ancestors.insert(F);
+  for (ModuleFile *Importer : F->ImportedBy)
+    getModuleFileAncestors(Importer, Ancestors);
+}
+
 void ModuleManager::removeModules(ModuleIterator first, ModuleIterator last,
                                   ModuleMap *modMap) {
   if (first == last)
     return;
 
-  // The first file entry is about to be rebuilt (or there was an error), so
-  // there should be no references to it. Remove it from the cache to close it,
-  // as Windows doesn't seem to allow renaming over an open file.
-  FileMgr.invalidateCache((*first)->File);
-
   // Collect the set of module file pointers that we'll be removing.
   llvm::SmallPtrSet<ModuleFile *, 4> victimSet(first, last);
 
+  // The last module file caused the load failure, so it and its ancestors in
+  // the module dependency tree will be rebuilt (or there was an error), so
+  // there should be no references to them. Collect the files to remove from
+  // the cache below, since rebuilding them will create new files at the old
+  // locations.
+  llvm::SmallPtrSet<ModuleFile *, 4> Ancestors;
+  getModuleFileAncestors(*(last-1), Ancestors);
+  assert(Ancestors.count(*first) && "non-dependent module loaded");
+
   // Remove any references to the now-destroyed modules.
   for (unsigned i = 0, n = Chain.size(); i != n; ++i) {
     Chain[i]->ImportedBy.remove_if([&](ModuleFile *MF) {
@@ -165,6 +177,10 @@ void ModuleManager::removeModules(Module
         mod->setASTFile(nullptr);
       }
     }
+
+    if (Ancestors.count(*victim))
+      FileMgr.invalidateCache((*victim)->File);
+
     delete *victim;
   }
 





More information about the cfe-commits mailing list