Skip to content

Commit 83f4c3a

Browse files
vsapsaivsapsai
committed
[modules] Do not cache invalid state for modules that we attempted to load.
Partially reverts 0a2be46 as it turned out to cause redundant module rebuilds in multi-process incremental builds. When a module was getting out of date, all compilation processes started at the same time were marking it as `ToBuild`. So each process was building the same module instead of checking if it was built by someone else and using that result. In addition to the work duplication, contention on the same .pcm file wasn't making builds faster. Note that for a single-process build this change would cause redundant module reads and validations. But reading a module is faster than building it and multi-process builds are more common than single-process. So I'm willing to make such a trade-off. rdar://problem/54395127 Reviewed By: dexonsmith Differential Revision: https://reviews.llvm.org/D72860
1 parent f55ab6f commit 83f4c3a

File tree

12 files changed

+37
-169
lines changed

12 files changed

+37
-169
lines changed

‎clang/include/clang/Serialization/InMemoryModuleCache.h

Lines changed: 8 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -45,61 +45,35 @@ class InMemoryModuleCache : public llvm::RefCountedBase<InMemoryModuleCache> {
4545
llvm::StringMap<PCM> PCMs;
4646

4747
public:
48-
/// There are four states for a PCM. It must monotonically increase.
49-
///
50-
/// 1. Unknown: the PCM has neither been read from disk nor built.
51-
/// 2. Tentative: the PCM has been read from disk but not yet imported or
52-
/// built. It might work.
53-
/// 3. ToBuild: the PCM read from disk did not work but a new one has not
54-
/// been built yet.
55-
/// 4. Final: indicating that the current PCM was either built in this
56-
/// process or has been successfully imported.
57-
enum State { Unknown, Tentative, ToBuild, Final };
58-
59-
/// Get the state of the PCM.
60-
State getPCMState(llvm::StringRef Filename) const;
61-
6248
/// Store the PCM under the Filename.
6349
///
64-
/// \pre state is Unknown
65-
/// \post state is Tentative
50+
/// \pre PCM for the same Filename shouldn't be in cache already.
6651
/// \return a reference to the buffer as a convenience.
6752
llvm::MemoryBuffer &addPCM(llvm::StringRef Filename,
6853
std::unique_ptr<llvm::MemoryBuffer> Buffer);
6954

70-
/// Store a just-built PCM under the Filename.
55+
/// Store a final PCM under the Filename.
7156
///
72-
/// \pre state is Unknown or ToBuild.
73-
/// \pre state is not Tentative.
57+
/// \pre PCM for the same Filename shouldn't be in cache already.
7458
/// \return a reference to the buffer as a convenience.
75-
llvm::MemoryBuffer &addBuiltPCM(llvm::StringRef Filename,
59+
llvm::MemoryBuffer &addFinalPCM(llvm::StringRef Filename,
7660
std::unique_ptr<llvm::MemoryBuffer> Buffer);
7761

78-
/// Try to remove a buffer from the cache. No effect if state is Final.
62+
/// Try to remove a PCM from the cache. No effect if it is Final.
7963
///
80-
/// \pre state is Tentative/Final.
81-
/// \post Tentative => ToBuild or Final => Final.
82-
/// \return false on success, i.e. if Tentative => ToBuild.
83-
bool tryToDropPCM(llvm::StringRef Filename);
64+
/// \return false on success.
65+
bool tryToRemovePCM(llvm::StringRef Filename);
8466

8567
/// Mark a PCM as final.
86-
///
87-
/// \pre state is Tentative or Final.
88-
/// \post state is Final.
8968
void finalizePCM(llvm::StringRef Filename);
9069

91-
/// Get a pointer to the pCM if it exists; else nullptr.
70+
/// Get a pointer to the PCM if it exists; else nullptr.
9271
llvm::MemoryBuffer *lookupPCM(llvm::StringRef Filename) const;
9372

9473
/// Check whether the PCM is final and has been shown to work.
9574
///
9675
/// \return true iff state is Final.
9776
bool isPCMFinal(llvm::StringRef Filename) const;
98-
99-
/// Check whether the PCM is waiting to be built.
100-
///
101-
/// \return true iff state is ToBuild.
102-
bool shouldBuildPCM(llvm::StringRef Filename) const;
10377
};
10478

10579
} // end namespace clang

‎clang/lib/Serialization/ASTReader.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4503,7 +4503,7 @@ ASTReader::ReadASTCore(StringRef FileName,
45034503
if (ShouldFinalizePCM)
45044504
MC.finalizePCM(FileName);
45054505
else
4506-
MC.tryToDropPCM(FileName);
4506+
MC.tryToRemovePCM(FileName);
45074507
});
45084508
ModuleFile &F = *M;
45094509
BitstreamCursor &Stream = F.Stream;

‎clang/lib/Serialization/ASTWriter.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4304,7 +4304,7 @@ ASTFileSignature ASTWriter::WriteAST(Sema &SemaRef,
43044304
WritingAST = false;
43054305
if (ShouldCacheASTInMemory) {
43064306
// Construct MemoryBuffer and update buffer manager.
4307-
ModuleCache.addBuiltPCM(OutputFile,
4307+
ModuleCache.addFinalPCM(OutputFile,
43084308
llvm::MemoryBuffer::getMemBufferCopy(
43094309
StringRef(Buffer.begin(), Buffer.size())));
43104310
}

‎clang/lib/Serialization/InMemoryModuleCache.cpp

Lines changed: 8 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -11,16 +11,6 @@
1111

1212
using namespace clang;
1313

14-
InMemoryModuleCache::State
15-
InMemoryModuleCache::getPCMState(llvm::StringRef Filename) const {
16-
auto I = PCMs.find(Filename);
17-
if (I == PCMs.end())
18-
return Unknown;
19-
if (I->second.IsFinal)
20-
return Final;
21-
return I->second.Buffer ? Tentative : ToBuild;
22-
}
23-
2414
llvm::MemoryBuffer &
2515
InMemoryModuleCache::addPCM(llvm::StringRef Filename,
2616
std::unique_ptr<llvm::MemoryBuffer> Buffer) {
@@ -30,11 +20,11 @@ InMemoryModuleCache::addPCM(llvm::StringRef Filename,
3020
}
3121

3222
llvm::MemoryBuffer &
33-
InMemoryModuleCache::addBuiltPCM(llvm::StringRef Filename,
23+
InMemoryModuleCache::addFinalPCM(llvm::StringRef Filename,
3424
std::unique_ptr<llvm::MemoryBuffer> Buffer) {
3525
auto &PCM = PCMs[Filename];
3626
assert(!PCM.IsFinal && "Trying to override finalized PCM?");
37-
assert(!PCM.Buffer && "Trying to override tentative PCM?");
27+
assert(!PCM.Buffer && "Already has a non-final PCM");
3828
PCM.Buffer = std::move(Buffer);
3929
PCM.IsFinal = true;
4030
return *PCM.Buffer;
@@ -49,24 +39,21 @@ InMemoryModuleCache::lookupPCM(llvm::StringRef Filename) const {
4939
}
5040

5141
bool InMemoryModuleCache::isPCMFinal(llvm::StringRef Filename) const {
52-
return getPCMState(Filename) == Final;
53-
}
54-
55-
bool InMemoryModuleCache::shouldBuildPCM(llvm::StringRef Filename) const {
56-
return getPCMState(Filename) == ToBuild;
42+
auto I = PCMs.find(Filename);
43+
if (I == PCMs.end())
44+
return false;
45+
return I->second.IsFinal;
5746
}
5847

59-
bool InMemoryModuleCache::tryToDropPCM(llvm::StringRef Filename) {
48+
bool InMemoryModuleCache::tryToRemovePCM(llvm::StringRef Filename) {
6049
auto I = PCMs.find(Filename);
6150
assert(I != PCMs.end() && "PCM to remove is unknown...");
6251

6352
auto &PCM = I->second;
64-
assert(PCM.Buffer && "PCM to remove is scheduled to be built...");
65-
6653
if (PCM.IsFinal)
6754
return true;
6855

69-
PCM.Buffer.reset();
56+
PCMs.erase(I);
7057
return false;
7158
}
7259

‎clang/lib/Serialization/ModuleManager.cpp

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ ModuleManager::addModule(StringRef FileName, ModuleKind Type,
163163
// Load the contents of the module
164164
if (std::unique_ptr<llvm::MemoryBuffer> Buffer = lookupBuffer(FileName)) {
165165
// The buffer was already provided for us.
166-
NewModule->Buffer = &ModuleCache->addBuiltPCM(FileName, std::move(Buffer));
166+
NewModule->Buffer = &ModuleCache->addFinalPCM(FileName, std::move(Buffer));
167167
// Since the cached buffer is reused, it is safe to close the file
168168
// descriptor that was opened while stat()ing the PCM in
169169
// lookupModuleFile() above, it won't be needed any longer.
@@ -173,19 +173,16 @@ ModuleManager::addModule(StringRef FileName, ModuleKind Type,
173173
NewModule->Buffer = Buffer;
174174
// As above, the file descriptor is no longer needed.
175175
Entry->closeFile();
176-
} else if (getModuleCache().shouldBuildPCM(FileName)) {
177-
// Report that the module is out of date, since we tried (and failed) to
178-
// import it earlier.
179-
Entry->closeFile();
180-
return OutOfDate;
181176
} else {
182177
// Open the AST file.
183178
llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> Buf((std::error_code()));
184179
if (FileName == "-") {
185180
Buf = llvm::MemoryBuffer::getSTDIN();
186181
} else {
187182
// Get a buffer of the file and close the file descriptor when done.
188-
Buf = FileMgr.getBufferForFile(NewModule->File, /*isVolatile=*/false);
183+
// The file is volatile because in a parallel build we expect multiple
184+
// compiler processes to use the same module file rebuilding it if needed.
185+
Buf = FileMgr.getBufferForFile(NewModule->File, /*isVolatile=*/true);
189186
}
190187

191188
if (!Buf) {

‎clang/test/Modules/Inputs/implicit-invalidate-chain/A.h

Lines changed: 0 additions & 2 deletions
This file was deleted.

‎clang/test/Modules/Inputs/implicit-invalidate-chain/B.h

Lines changed: 0 additions & 2 deletions
This file was deleted.

‎clang/test/Modules/Inputs/implicit-invalidate-chain/C.h

Lines changed: 0 additions & 2 deletions
This file was deleted.

‎clang/test/Modules/Inputs/implicit-invalidate-chain/module.modulemap

Lines changed: 0 additions & 3 deletions
This file was deleted.

‎clang/test/Modules/implicit-invalidate-chain.c

Lines changed: 0 additions & 67 deletions
This file was deleted.

0 commit comments

Comments
 (0)