summaryrefslogtreecommitdiffstats
path: root/lib/Serialization/ASTReader.cpp
diff options
context:
space:
mode:
authorDuncan P. N. Exon Smith <dexonsmith@apple.com>2019-03-09 17:44:01 +0000
committerDuncan P. N. Exon Smith <dexonsmith@apple.com>2019-03-09 17:44:01 +0000
commitc977c99e5c50af2baad41e98e5369ffb7d7685c9 (patch)
tree4a8c27e10ecdc54255bc8821f90a20fc25aa69c7 /lib/Serialization/ASTReader.cpp
parentd28cf14aed192f83a38af4a07e6c14af97946947 (diff)
Modules: Invalidate out-of-date PCMs as they're discovered
Leverage the InMemoryModuleCache to invalidate a module the first time it fails to import (and to lock a module as soon as it's built or imported successfully). For implicit module builds, this optimizes importing deep graphs where the leaf module is out-of-date; see example near the end of the commit message. Previously the cache finalized ("locked in") all modules imported so far when starting a new module build. This was sufficient to prevent loading two versions of the same module, but was somewhat arbitrary and hard to reason about. Now the cache explicitly tracks module state, where each module must be one of: - Unknown: module not in the cache (yet). - Tentative: module in the cache, but not yet fully imported. - ToBuild: module found on disk could not be imported; need to build. - Final: module in the cache has been successfully built or imported. Preventing repeated failed imports avoids variation in builds based on shifting filesystem state. Now it's guaranteed that a module is loaded from disk exactly once. It now seems safe to remove FileManager::invalidateCache, but I'm leaving that for a later commit. The new, precise logic uncovered a pre-existing problem in the cache: the map key is the module filename, and different contexts use different filenames for the same PCM file. (In particular, the test Modules/relative-import-path.c does not build without this commit. r223577 started using a relative path to describe a module's base directory when importing it within another module. As a result, the module cache sees an absolute path when (a) building the module or importing it at the top-level, and a relative path when (b) importing the module underneath another one.) The "obvious" fix is to resolve paths using FileManager::getVirtualFile and change the map key for the cache to a FileEntry, but some contexts (particularly related to ASTUnit) have a shorter lifetime for their FileManager than the InMemoryModuleCache. This is worth pursuing further in a later commit; perhaps by tying together the FileManager and InMemoryModuleCache lifetime, or moving the in-memory PCM storage into a VFS layer. For now, use the PCM's base directory as-written for constructing the filename to check the ModuleCache. Example ======= To understand the build optimization, first consider the build of a module graph TU -> A -> B -> C -> D with an empty cache: TU builds A' A' builds B' B' builds C' C' builds D' imports D' B' imports C' imports D' A' imports B' imports C' imports D' TU imports A' imports B' imports C' imports D' If we build TU again, where A, B, C, and D are in the cache and D is out-of-date, we would previously get this build: TU imports A imports B imports C imports D (out-of-date) TU builds A' A' imports B imports C imports D (out-of-date) builds B' B' imports C imports D (out-of-date) builds C' C' imports D (out-of-date) builds D' imports D' B' imports C' imports D' A' imports B' imports C' imports D' TU imports A' imports B' imports C' imports D' After this commit, we'll immediateley invalidate A, B, C, and D when we first observe that D is out-of-date, giving this build: TU imports A imports B imports C imports D (out-of-date) TU builds A' // The same graph as an empty cache. A' builds B' B' builds C' C' builds D' imports D' B' imports C' imports D' A' imports B' imports C' imports D' TU imports A' imports B' imports C' imports D' The new build matches what we'd naively expect, pretty closely matching the original build with the empty cache. rdar://problem/48545366 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@355778 91177308-0d34-0410-b5e6-96231b3b80d8
Diffstat (limited to 'lib/Serialization/ASTReader.cpp')
-rw-r--r--lib/Serialization/ASTReader.cpp28
1 files changed, 26 insertions, 2 deletions
diff --git a/lib/Serialization/ASTReader.cpp b/lib/Serialization/ASTReader.cpp
index b8b390afc4..1ab6a75f8c 100644
--- a/lib/Serialization/ASTReader.cpp
+++ b/lib/Serialization/ASTReader.cpp
@@ -92,6 +92,7 @@
#include "llvm/ADT/None.h"
#include "llvm/ADT/Optional.h"
#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/ScopeExit.h"
#include "llvm/ADT/SmallPtrSet.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/SmallVector.h"
@@ -2359,6 +2360,7 @@ ASTReader::ReadControlBlock(ModuleFile &F,
RecordData Record;
unsigned NumInputs = 0;
unsigned NumUserInputs = 0;
+ StringRef BaseDirectoryAsWritten;
while (true) {
llvm::BitstreamEntry Entry = Stream.advance();
@@ -2559,7 +2561,9 @@ ASTReader::ReadControlBlock(ModuleFile &F,
ImportedName, /*FileMapOnly*/ true);
if (ImportedFile.empty())
- ImportedFile = ReadPath(F, Record, Idx);
+ // Use BaseDirectoryAsWritten to ensure we use the same path in the
+ // ModuleCache as when writing.
+ ImportedFile = ReadPath(BaseDirectoryAsWritten, Record, Idx);
else
SkipPath(Record, Idx);
@@ -2624,6 +2628,9 @@ ASTReader::ReadControlBlock(ModuleFile &F,
break;
case MODULE_DIRECTORY: {
+ // Save the BaseDirectory as written in the PCM for computing the module
+ // filename for the ModuleCache.
+ BaseDirectoryAsWritten = Blob;
assert(!F.ModuleName.empty() &&
"MODULE_DIRECTORY found before MODULE_NAME");
// If we've already loaded a module map file covering this module, we may
@@ -4180,6 +4187,14 @@ ASTReader::ReadASTCore(StringRef FileName,
assert(M && "Missing module file");
+ bool ShouldFinalizePCM = false;
+ auto FinalizeOrDropPCM = llvm::make_scope_exit([&]() {
+ auto &MC = getModuleManager().getModuleCache();
+ if (ShouldFinalizePCM)
+ MC.finalizePCM(FileName);
+ else
+ MC.tryToDropPCM(FileName);
+ });
ModuleFile &F = *M;
BitstreamCursor &Stream = F.Stream;
Stream = BitstreamCursor(PCHContainerRdr.ExtractPCH(*F.Buffer));
@@ -4246,6 +4261,7 @@ ASTReader::ReadASTCore(StringRef FileName,
// Record that we've loaded this module.
Loaded.push_back(ImportedModule(M, ImportedBy, ImportLoc));
+ ShouldFinalizePCM = true;
return Success;
case UNHASHED_CONTROL_BLOCK_ID:
@@ -4309,7 +4325,7 @@ ASTReader::readUnhashedControlBlock(ModuleFile &F, bool WasImportedBy,
// validation will fail during the as-system import since the PCM on disk
// doesn't guarantee that -Werror was respected. However, the -Werror
// flags were checked during the initial as-user import.
- if (getModuleManager().getModuleCache().isBufferFinal(F.FileName)) {
+ if (getModuleManager().getModuleCache().isPCMFinal(F.FileName)) {
Diag(diag::warn_module_system_bit_conflict) << F.FileName;
return Success;
}
@@ -9099,6 +9115,14 @@ std::string ASTReader::ReadPath(ModuleFile &F, const RecordData &Record,
return Filename;
}
+std::string ASTReader::ReadPath(StringRef BaseDirectory,
+ const RecordData &Record, unsigned &Idx) {
+ std::string Filename = ReadString(Record, Idx);
+ if (!BaseDirectory.empty())
+ ResolveImportedPath(Filename, BaseDirectory);
+ return Filename;
+}
+
VersionTuple ASTReader::ReadVersionTuple(const RecordData &Record,
unsigned &Idx) {
unsigned Major = Record[Idx++];