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

[Ben Langmuir]

Could you also approve this for the seed in the radar?  I’ve put in the CCC info.

Thanks,

Ben

On May 30, 2014, at 3:32 PM, Argyrios Kyrtzidis <kyrtzidis at apple.com> wrote:

> LGTM!
> 
>> On May 30, 2014, at 2:20 PM, Ben Langmuir <blangmuir at apple.com> wrote:
>> 
>> 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;
>>  }
>> 
>> 
>> 
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>