Modules: Cache PCMs in memory and avoid a use-after-free
Clang's internal build system for implicit modules uses lock files to
ensure that after a process writes a PCM it will read the same one back
in (without contention from other -cc1 commands). Since PCMs are read
from disk repeatedly while invalidating, building, and importing, the
lock is not released quickly. Furthermore, the LockFileManager is not
robust in every environment. Other -cc1 commands can stall until
timeout (after about eight minutes).
This commit changes the lock file from being necessary for correctness
to a (possibly dubious) performance hack. The remaining benefit is to
reduce duplicate work in competing -cc1 commands which depend on the
same module. Follow-up commits will change the internal build system to
continue after a timeout, and reduce the timeout. Perhaps we should
reconsider blocking at all.
This also fixes a use-after-free, when one part of a compilation
validates a PCM and starts using it, and another tries to swap out the
PCM for something new.
The PCMCache is a new type called MemoryBufferCache, which saves memory
buffers based on their filename. Its ownership is shared by the
CompilerInstance and ModuleManager.
- The ModuleManager stores PCMs there that it loads from disk, never
touching the disk if the cache is hot.
- When modules fail to validate, they're removed from the cache.
- When a CompilerInstance is spawned to build a new module, each
already-loaded PCM is assumed to be valid, and is frozen to avoid
the use-after-free.
- Any newly-built module is written directly to the cache to avoid the
round-trip to the filesystem, making lock files unnecessary for
correctness.
Original patch by Manman Ren; most testcases by Adrian Prantl!
llvm-svn: 298165
diff --git a/clang/unittests/Basic/CMakeLists.txt b/clang/unittests/Basic/CMakeLists.txt
index 3cb3cb8..3a9f34f 100644
--- a/clang/unittests/Basic/CMakeLists.txt
+++ b/clang/unittests/Basic/CMakeLists.txt
@@ -6,6 +6,7 @@
CharInfoTest.cpp
DiagnosticTest.cpp
FileManagerTest.cpp
+ MemoryBufferCacheTest.cpp
SourceManagerTest.cpp
VirtualFileSystemTest.cpp
)
diff --git a/clang/unittests/Basic/MemoryBufferCacheTest.cpp b/clang/unittests/Basic/MemoryBufferCacheTest.cpp
new file mode 100644
index 0000000..be243c2
--- /dev/null
+++ b/clang/unittests/Basic/MemoryBufferCacheTest.cpp
@@ -0,0 +1,94 @@
+//===- MemoryBufferCacheTest.cpp - MemoryBufferCache tests ----------------===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang/Basic/MemoryBufferCache.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "gtest/gtest.h"
+
+using namespace llvm;
+using namespace clang;
+
+namespace {
+
+std::unique_ptr<MemoryBuffer> getBuffer(int I) {
+ SmallVector<char, 8> Bytes;
+ raw_svector_ostream(Bytes) << "data:" << I;
+ return MemoryBuffer::getMemBuffer(StringRef(Bytes.data(), Bytes.size()));
+}
+
+TEST(MemoryBufferCacheTest, addBuffer) {
+ auto B1 = getBuffer(1);
+ auto B2 = getBuffer(2);
+ auto B3 = getBuffer(3);
+ auto *RawB1 = B1.get();
+ auto *RawB2 = B2.get();
+ auto *RawB3 = B3.get();
+
+ // Add a few buffers.
+ MemoryBufferCache Cache;
+ EXPECT_EQ(RawB1, &Cache.addBuffer("1", std::move(B1)));
+ EXPECT_EQ(RawB2, &Cache.addBuffer("2", std::move(B2)));
+ EXPECT_EQ(RawB3, &Cache.addBuffer("3", std::move(B3)));
+ EXPECT_EQ(RawB1, Cache.lookupBuffer("1"));
+ EXPECT_EQ(RawB2, Cache.lookupBuffer("2"));
+ EXPECT_EQ(RawB3, Cache.lookupBuffer("3"));
+ EXPECT_FALSE(Cache.isBufferFinal("1"));
+ EXPECT_FALSE(Cache.isBufferFinal("2"));
+ EXPECT_FALSE(Cache.isBufferFinal("3"));
+
+ // Remove the middle buffer.
+ EXPECT_FALSE(Cache.tryToRemoveBuffer("2"));
+ EXPECT_EQ(nullptr, Cache.lookupBuffer("2"));
+ EXPECT_FALSE(Cache.isBufferFinal("2"));
+
+ // Replace the middle buffer.
+ B2 = getBuffer(2);
+ ASSERT_NE(RawB2, B2.get());
+ RawB2 = B2.get();
+ EXPECT_EQ(RawB2, &Cache.addBuffer("2", std::move(B2)));
+
+ // Check that nothing is final.
+ EXPECT_FALSE(Cache.isBufferFinal("1"));
+ EXPECT_FALSE(Cache.isBufferFinal("2"));
+ EXPECT_FALSE(Cache.isBufferFinal("3"));
+}
+
+TEST(MemoryBufferCacheTest, finalizeCurrentBuffers) {
+ // Add a buffer.
+ MemoryBufferCache Cache;
+ auto B1 = getBuffer(1);
+ auto *RawB1 = B1.get();
+ Cache.addBuffer("1", std::move(B1));
+ ASSERT_FALSE(Cache.isBufferFinal("1"));
+
+ // Finalize it.
+ Cache.finalizeCurrentBuffers();
+ EXPECT_TRUE(Cache.isBufferFinal("1"));
+ EXPECT_TRUE(Cache.tryToRemoveBuffer("1"));
+ EXPECT_EQ(RawB1, Cache.lookupBuffer("1"));
+ EXPECT_TRUE(Cache.isBufferFinal("1"));
+
+ // Repeat.
+ auto B2 = getBuffer(2);
+ auto *RawB2 = B2.get();
+ Cache.addBuffer("2", std::move(B2));
+ EXPECT_FALSE(Cache.isBufferFinal("2"));
+
+ Cache.finalizeCurrentBuffers();
+ EXPECT_TRUE(Cache.isBufferFinal("1"));
+ EXPECT_TRUE(Cache.isBufferFinal("2"));
+ EXPECT_TRUE(Cache.tryToRemoveBuffer("1"));
+ EXPECT_TRUE(Cache.tryToRemoveBuffer("2"));
+ EXPECT_EQ(RawB1, Cache.lookupBuffer("1"));
+ EXPECT_EQ(RawB2, Cache.lookupBuffer("2"));
+ EXPECT_TRUE(Cache.isBufferFinal("1"));
+ EXPECT_TRUE(Cache.isBufferFinal("2"));
+}
+
+} // namespace
diff --git a/clang/unittests/Basic/SourceManagerTest.cpp b/clang/unittests/Basic/SourceManagerTest.cpp
index a967b0e..dddc3f9 100644
--- a/clang/unittests/Basic/SourceManagerTest.cpp
+++ b/clang/unittests/Basic/SourceManagerTest.cpp
@@ -12,6 +12,7 @@
#include "clang/Basic/DiagnosticOptions.h"
#include "clang/Basic/FileManager.h"
#include "clang/Basic/LangOptions.h"
+#include "clang/Basic/MemoryBufferCache.h"
#include "clang/Basic/TargetInfo.h"
#include "clang/Basic/TargetOptions.h"
#include "clang/Lex/HeaderSearch.h"
@@ -78,10 +79,11 @@
SourceMgr.setMainFileID(mainFileID);
VoidModuleLoader ModLoader;
+ MemoryBufferCache PCMCache;
HeaderSearch HeaderInfo(std::make_shared<HeaderSearchOptions>(), SourceMgr,
Diags, LangOpts, &*Target);
Preprocessor PP(std::make_shared<PreprocessorOptions>(), Diags, LangOpts,
- SourceMgr, HeaderInfo, ModLoader,
+ SourceMgr, PCMCache, HeaderInfo, ModLoader,
/*IILookup =*/nullptr,
/*OwnsHeaderSearch =*/false);
PP.Initialize(*Target);
@@ -198,10 +200,11 @@
SourceMgr.overrideFileContents(headerFile, std::move(HeaderBuf));
VoidModuleLoader ModLoader;
+ MemoryBufferCache PCMCache;
HeaderSearch HeaderInfo(std::make_shared<HeaderSearchOptions>(), SourceMgr,
Diags, LangOpts, &*Target);
Preprocessor PP(std::make_shared<PreprocessorOptions>(), Diags, LangOpts,
- SourceMgr, HeaderInfo, ModLoader,
+ SourceMgr, PCMCache, HeaderInfo, ModLoader,
/*IILookup =*/nullptr,
/*OwnsHeaderSearch =*/false);
PP.Initialize(*Target);
@@ -298,10 +301,11 @@
SourceMgr.overrideFileContents(headerFile, std::move(HeaderBuf));
VoidModuleLoader ModLoader;
+ MemoryBufferCache PCMCache;
HeaderSearch HeaderInfo(std::make_shared<HeaderSearchOptions>(), SourceMgr,
Diags, LangOpts, &*Target);
Preprocessor PP(std::make_shared<PreprocessorOptions>(), Diags, LangOpts,
- SourceMgr, HeaderInfo, ModLoader,
+ SourceMgr, PCMCache, HeaderInfo, ModLoader,
/*IILookup =*/nullptr,
/*OwnsHeaderSearch =*/false);
PP.Initialize(*Target);
diff --git a/clang/unittests/Lex/LexerTest.cpp b/clang/unittests/Lex/LexerTest.cpp
index 918167b..b5a6fd9 100644
--- a/clang/unittests/Lex/LexerTest.cpp
+++ b/clang/unittests/Lex/LexerTest.cpp
@@ -12,6 +12,7 @@
#include "clang/Basic/DiagnosticOptions.h"
#include "clang/Basic/FileManager.h"
#include "clang/Basic/LangOptions.h"
+#include "clang/Basic/MemoryBufferCache.h"
#include "clang/Basic/SourceManager.h"
#include "clang/Basic/TargetInfo.h"
#include "clang/Basic/TargetOptions.h"
@@ -64,10 +65,12 @@
SourceMgr.setMainFileID(SourceMgr.createFileID(std::move(Buf)));
VoidModuleLoader ModLoader;
+ MemoryBufferCache PCMCache;
HeaderSearch HeaderInfo(std::make_shared<HeaderSearchOptions>(), SourceMgr,
Diags, LangOpts, Target.get());
Preprocessor PP(std::make_shared<PreprocessorOptions>(), Diags, LangOpts,
- SourceMgr, HeaderInfo, ModLoader, /*IILookup =*/nullptr,
+ SourceMgr, PCMCache, HeaderInfo, ModLoader,
+ /*IILookup =*/nullptr,
/*OwnsHeaderSearch =*/false);
PP.Initialize(*Target);
PP.EnterMainSourceFile();
diff --git a/clang/unittests/Lex/PPCallbacksTest.cpp b/clang/unittests/Lex/PPCallbacksTest.cpp
index 064abaf..0841791 100644
--- a/clang/unittests/Lex/PPCallbacksTest.cpp
+++ b/clang/unittests/Lex/PPCallbacksTest.cpp
@@ -14,6 +14,7 @@
#include "clang/Basic/DiagnosticOptions.h"
#include "clang/Basic/FileManager.h"
#include "clang/Basic/LangOptions.h"
+#include "clang/Basic/MemoryBufferCache.h"
#include "clang/Basic/SourceManager.h"
#include "clang/Basic/TargetInfo.h"
#include "clang/Basic/TargetOptions.h"
@@ -161,13 +162,14 @@
SourceMgr.setMainFileID(SourceMgr.createFileID(std::move(Buf)));
VoidModuleLoader ModLoader;
+ MemoryBufferCache PCMCache;
HeaderSearch HeaderInfo(std::make_shared<HeaderSearchOptions>(), SourceMgr,
Diags, LangOpts, Target.get());
AddFakeHeader(HeaderInfo, HeaderPath, SystemHeader);
Preprocessor PP(std::make_shared<PreprocessorOptions>(), Diags, LangOpts,
- SourceMgr, HeaderInfo, ModLoader,
+ SourceMgr, PCMCache, HeaderInfo, ModLoader,
/*IILookup =*/nullptr,
/*OwnsHeaderSearch =*/false);
PP.Initialize(*Target);
@@ -198,11 +200,12 @@
SourceMgr.setMainFileID(SourceMgr.createFileID(std::move(SourceBuf)));
VoidModuleLoader ModLoader;
+ MemoryBufferCache PCMCache;
HeaderSearch HeaderInfo(std::make_shared<HeaderSearchOptions>(), SourceMgr,
Diags, OpenCLLangOpts, Target.get());
Preprocessor PP(std::make_shared<PreprocessorOptions>(), Diags,
- OpenCLLangOpts, SourceMgr, HeaderInfo, ModLoader,
+ OpenCLLangOpts, SourceMgr, PCMCache, HeaderInfo, ModLoader,
/*IILookup =*/nullptr,
/*OwnsHeaderSearch =*/false);
PP.Initialize(*Target);
diff --git a/clang/unittests/Lex/PPConditionalDirectiveRecordTest.cpp b/clang/unittests/Lex/PPConditionalDirectiveRecordTest.cpp
index dccfffd..b635604 100644
--- a/clang/unittests/Lex/PPConditionalDirectiveRecordTest.cpp
+++ b/clang/unittests/Lex/PPConditionalDirectiveRecordTest.cpp
@@ -12,6 +12,7 @@
#include "clang/Basic/DiagnosticOptions.h"
#include "clang/Basic/FileManager.h"
#include "clang/Basic/LangOptions.h"
+#include "clang/Basic/MemoryBufferCache.h"
#include "clang/Basic/SourceManager.h"
#include "clang/Basic/TargetInfo.h"
#include "clang/Basic/TargetOptions.h"
@@ -93,10 +94,11 @@
SourceMgr.setMainFileID(SourceMgr.createFileID(std::move(Buf)));
VoidModuleLoader ModLoader;
+ MemoryBufferCache PCMCache;
HeaderSearch HeaderInfo(std::make_shared<HeaderSearchOptions>(), SourceMgr,
Diags, LangOpts, Target.get());
Preprocessor PP(std::make_shared<PreprocessorOptions>(), Diags, LangOpts,
- SourceMgr, HeaderInfo, ModLoader,
+ SourceMgr, PCMCache, HeaderInfo, ModLoader,
/*IILookup =*/nullptr,
/*OwnsHeaderSearch =*/false);
PP.Initialize(*Target);