[clang][deps] Keep module cache in memory#192347
Conversation
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
ad2abca to
a5629bf
Compare
|
@llvm/pr-subscribers-clang Author: Jan Svoboda (jansvoboda11) ChangesWith this PR, module cache PCMs are kept in-memory and can be flushed to disk at the end of the build if desired. Not going to disk at all speeds up dependency scans by around 4.7% by removing a source of contention in the kernel. Full diff: https://github.com/llvm/llvm-project/pull/192347.diff 4 Files Affected:
diff --git a/clang/include/clang/DependencyScanning/InProcessModuleCache.h b/clang/include/clang/DependencyScanning/InProcessModuleCache.h
index 8ad9811606611..72d03c94a1488 100644
--- a/clang/include/clang/DependencyScanning/InProcessModuleCache.h
+++ b/clang/include/clang/DependencyScanning/InProcessModuleCache.h
@@ -11,11 +11,17 @@
#include "clang/Serialization/ModuleCache.h"
#include "llvm/ADT/StringMap.h"
+#include "llvm/ADT/StringSet.h"
#include <atomic>
#include <condition_variable>
+#include <memory>
#include <mutex>
+namespace llvm {
+class MemoryBuffer;
+} // namespace llvm
+
namespace clang {
namespace dependencies {
@@ -26,11 +32,18 @@ struct ModuleCacheEntry {
unsigned Generation = 0;
std::atomic<std::time_t> Timestamp = 0;
+
+ std::unique_ptr<llvm::MemoryBuffer> Buffer;
+ time_t ModTime = 0;
+ bool NeedsFlush = false;
};
struct ModuleCacheEntries {
std::mutex Mutex;
llvm::StringMap<std::unique_ptr<ModuleCacheEntry>> Map;
+
+ /// Flushes all PCMs built in-process-built to disk.
+ void flush();
};
std::shared_ptr<ModuleCache>
diff --git a/clang/lib/DependencyScanning/InProcessModuleCache.cpp b/clang/lib/DependencyScanning/InProcessModuleCache.cpp
index 1f9a8e2f85c02..b52e4ab822f38 100644
--- a/clang/lib/DependencyScanning/InProcessModuleCache.cpp
+++ b/clang/lib/DependencyScanning/InProcessModuleCache.cpp
@@ -12,11 +12,28 @@
#include "llvm/Support/AdvisoryLock.h"
#include "llvm/Support/Chrono.h"
#include "llvm/Support/Error.h"
+#include "llvm/Support/FileSystem.h"
#include "llvm/Support/IOSandbox.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/Path.h"
using namespace clang;
using namespace dependencies;
+void ModuleCacheEntries::flush() {
+ // Flush all PCMs that were built in-process to disk so they are available
+ // for subsequent builds.
+ auto BypassSandbox = llvm::sys::sandbox::scopedDisable();
+ for (auto &[Path, Entry] : Map) {
+ if (!Entry->NeedsFlush)
+ continue;
+ off_t Size;
+ time_t ModTime;
+ // Best-effort: ignore errors (e.g. read-only cache directory).
+ (void)writeImpl(Path, Entry->Buffer->getMemBufferRef(), Size, ModTime);
+ }
+}
+
namespace {
class ReaderWriterLock : public llvm::AdvisoryLock {
ModuleCacheEntry &Entry;
@@ -81,43 +98,29 @@ class InProcessModuleCache : public ModuleCache {
// recreated for each translation unit.
InMemoryModuleCache InMemory;
+ ModuleCacheEntry &getOrCreateEntry(StringRef Filename) {
+ std::lock_guard<std::mutex> Lock(Entries.Mutex);
+ auto &Entry = Entries.Map[Filename];
+ if (!Entry)
+ Entry = std::make_unique<ModuleCacheEntry>();
+ return *Entry;
+ }
+
public:
InProcessModuleCache(ModuleCacheEntries &Entries) : Entries(Entries) {}
std::unique_ptr<llvm::AdvisoryLock> getLock(StringRef Filename) override {
- auto &Entry = [&]() -> ModuleCacheEntry & {
- std::lock_guard<std::mutex> Lock(Entries.Mutex);
- auto &Entry = Entries.Map[Filename];
- if (!Entry)
- Entry = std::make_unique<ModuleCacheEntry>();
- return *Entry;
- }();
- return std::make_unique<ReaderWriterLock>(Entry);
+ return std::make_unique<ReaderWriterLock>(getOrCreateEntry(Filename));
}
std::time_t getModuleTimestamp(StringRef Filename) override {
- auto &Timestamp = [&]() -> std::atomic<std::time_t> & {
- std::lock_guard<std::mutex> Lock(Entries.Mutex);
- auto &Entry = Entries.Map[Filename];
- if (!Entry)
- Entry = std::make_unique<ModuleCacheEntry>();
- return Entry->Timestamp;
- }();
-
- return Timestamp.load();
+ return getOrCreateEntry(Filename).Timestamp.load();
}
void updateModuleTimestamp(StringRef Filename) override {
// Note: This essentially replaces FS contention with mutex contention.
- auto &Timestamp = [&]() -> std::atomic<std::time_t> & {
- std::lock_guard<std::mutex> Lock(Entries.Mutex);
- auto &Entry = Entries.Map[Filename];
- if (!Entry)
- Entry = std::make_unique<ModuleCacheEntry>();
- return Entry->Timestamp;
- }();
-
- Timestamp.store(llvm::sys::toTimeT(std::chrono::system_clock::now()));
+ getOrCreateEntry(Filename).Timestamp.store(
+ llvm::sys::toTimeT(std::chrono::system_clock::now()));
}
void maybePrune(StringRef Path, time_t PruneInterval,
@@ -134,12 +137,13 @@ class InProcessModuleCache : public ModuleCache {
std::error_code write(StringRef Path, llvm::MemoryBufferRef Buffer,
off_t &Size, time_t &ModTime) override {
- // This is a compiler-internal input/output, let's bypass the sandbox.
- auto BypassSandbox = llvm::sys::sandbox::scopedDisable();
-
- // FIXME: This could use an in-memory cache to avoid IO, and only write to
- // disk at the end of the scan.
- return writeImpl(Path, Buffer, Size, ModTime);
+ ModuleCacheEntry &Entry = getOrCreateEntry(Path);
+ std::lock_guard<std::mutex> Lock(Entry.Mutex);
+ Entry.Buffer =
+ llvm::MemoryBuffer::getMemBufferCopy(Buffer.getBuffer(), Path);
+ Entry.ModTime = llvm::sys::toTimeT(std::chrono::system_clock::now());
+ Entry.NeedsFlush = true;
+ return {};
}
Expected<std::unique_ptr<llvm::MemoryBuffer>>
@@ -147,9 +151,21 @@ class InProcessModuleCache : public ModuleCache {
// This is a compiler-internal input/output, let's bypass the sandbox.
auto BypassSandbox = llvm::sys::sandbox::scopedDisable();
- // FIXME: This only needs to go to disk once per build, not in every
- // compilation. Introduce in-memory cache.
- return readImpl(FileName, Size, ModTime);
+ ModuleCacheEntry &Entry = getOrCreateEntry(FileName);
+ std::lock_guard<std::mutex> Lock(Entry.Mutex);
+ if (!Entry.Buffer) {
+ off_t ReadSize;
+ time_t ReadModTime;
+ auto ReadBuffer = readImpl(FileName, ReadSize, ReadModTime);
+ if (!ReadBuffer)
+ return ReadBuffer.takeError();
+ Entry.Buffer = std::move(*ReadBuffer);
+ Entry.ModTime = ReadModTime;
+ Entry.NeedsFlush = false;
+ }
+ Size = Entry.Buffer->getBufferSize();
+ ModTime = Entry.ModTime;
+ return llvm::MemoryBuffer::getMemBuffer(*Entry.Buffer);
}
};
} // namespace
diff --git a/clang/lib/Serialization/ModuleCache.cpp b/clang/lib/Serialization/ModuleCache.cpp
index efbaf9765546f..a61724819530a 100644
--- a/clang/lib/Serialization/ModuleCache.cpp
+++ b/clang/lib/Serialization/ModuleCache.cpp
@@ -24,9 +24,22 @@ const ModuleCacheDirectory *ModuleCache::getDirectoryPtr(StringRef Path) {
if (!ByNameIt->second) {
// This is a compiler-internal input/output, let's bypass the sandbox.
auto BypassSandbox = llvm::sys::sandbox::scopedDisable();
+
+ // If we cannot get status of the module cache directory, try if trying to
+ // create it helps.
llvm::sys::fs::file_status Status;
- if (llvm::sys::fs::status(Path, Status))
- return nullptr;
+ if (std::error_code EC = llvm::sys::fs::status(Path, Status)) {
+ // Unless the status failed because the directory does not exist yet.
+ if (EC != std::errc::no_such_file_or_directory)
+ return nullptr;
+ // If we're unable to create the directory.
+ if (llvm::sys::fs::create_directories(Path))
+ return nullptr;
+ // If we're unable to stat the newly created directory.
+ if (llvm::sys::fs::status(Path, Status))
+ return nullptr;
+ }
+
llvm::sys::fs::UniqueID UID = Status.getUniqueID();
auto [ByUIDIt, ByUIDInserted] = ByUID.insert({UID, nullptr});
if (!ByUIDIt->second)
diff --git a/clang/tools/clang-scan-deps/ClangScanDeps.cpp b/clang/tools/clang-scan-deps/ClangScanDeps.cpp
index fd3d70f9498ee..54343df29efe1 100644
--- a/clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ b/clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -1159,6 +1159,8 @@ int clang_scan_deps_main(int argc, char **argv, const llvm::ToolContext &) {
Pool.wait();
}
+ Service.getModuleCacheEntries().flush();
+
T.stopTimer();
if (Verbose)
|
|
@llvm/pr-subscribers-clang-modules Author: Jan Svoboda (jansvoboda11) ChangesWith this PR, module cache PCMs are kept in-memory and can be flushed to disk at the end of the build if desired. Not going to disk at all speeds up dependency scans by around 4.7% by removing a source of contention in the kernel. Full diff: https://github.com/llvm/llvm-project/pull/192347.diff 4 Files Affected:
diff --git a/clang/include/clang/DependencyScanning/InProcessModuleCache.h b/clang/include/clang/DependencyScanning/InProcessModuleCache.h
index 8ad9811606611..72d03c94a1488 100644
--- a/clang/include/clang/DependencyScanning/InProcessModuleCache.h
+++ b/clang/include/clang/DependencyScanning/InProcessModuleCache.h
@@ -11,11 +11,17 @@
#include "clang/Serialization/ModuleCache.h"
#include "llvm/ADT/StringMap.h"
+#include "llvm/ADT/StringSet.h"
#include <atomic>
#include <condition_variable>
+#include <memory>
#include <mutex>
+namespace llvm {
+class MemoryBuffer;
+} // namespace llvm
+
namespace clang {
namespace dependencies {
@@ -26,11 +32,18 @@ struct ModuleCacheEntry {
unsigned Generation = 0;
std::atomic<std::time_t> Timestamp = 0;
+
+ std::unique_ptr<llvm::MemoryBuffer> Buffer;
+ time_t ModTime = 0;
+ bool NeedsFlush = false;
};
struct ModuleCacheEntries {
std::mutex Mutex;
llvm::StringMap<std::unique_ptr<ModuleCacheEntry>> Map;
+
+ /// Flushes all PCMs built in-process-built to disk.
+ void flush();
};
std::shared_ptr<ModuleCache>
diff --git a/clang/lib/DependencyScanning/InProcessModuleCache.cpp b/clang/lib/DependencyScanning/InProcessModuleCache.cpp
index 1f9a8e2f85c02..b52e4ab822f38 100644
--- a/clang/lib/DependencyScanning/InProcessModuleCache.cpp
+++ b/clang/lib/DependencyScanning/InProcessModuleCache.cpp
@@ -12,11 +12,28 @@
#include "llvm/Support/AdvisoryLock.h"
#include "llvm/Support/Chrono.h"
#include "llvm/Support/Error.h"
+#include "llvm/Support/FileSystem.h"
#include "llvm/Support/IOSandbox.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/Path.h"
using namespace clang;
using namespace dependencies;
+void ModuleCacheEntries::flush() {
+ // Flush all PCMs that were built in-process to disk so they are available
+ // for subsequent builds.
+ auto BypassSandbox = llvm::sys::sandbox::scopedDisable();
+ for (auto &[Path, Entry] : Map) {
+ if (!Entry->NeedsFlush)
+ continue;
+ off_t Size;
+ time_t ModTime;
+ // Best-effort: ignore errors (e.g. read-only cache directory).
+ (void)writeImpl(Path, Entry->Buffer->getMemBufferRef(), Size, ModTime);
+ }
+}
+
namespace {
class ReaderWriterLock : public llvm::AdvisoryLock {
ModuleCacheEntry &Entry;
@@ -81,43 +98,29 @@ class InProcessModuleCache : public ModuleCache {
// recreated for each translation unit.
InMemoryModuleCache InMemory;
+ ModuleCacheEntry &getOrCreateEntry(StringRef Filename) {
+ std::lock_guard<std::mutex> Lock(Entries.Mutex);
+ auto &Entry = Entries.Map[Filename];
+ if (!Entry)
+ Entry = std::make_unique<ModuleCacheEntry>();
+ return *Entry;
+ }
+
public:
InProcessModuleCache(ModuleCacheEntries &Entries) : Entries(Entries) {}
std::unique_ptr<llvm::AdvisoryLock> getLock(StringRef Filename) override {
- auto &Entry = [&]() -> ModuleCacheEntry & {
- std::lock_guard<std::mutex> Lock(Entries.Mutex);
- auto &Entry = Entries.Map[Filename];
- if (!Entry)
- Entry = std::make_unique<ModuleCacheEntry>();
- return *Entry;
- }();
- return std::make_unique<ReaderWriterLock>(Entry);
+ return std::make_unique<ReaderWriterLock>(getOrCreateEntry(Filename));
}
std::time_t getModuleTimestamp(StringRef Filename) override {
- auto &Timestamp = [&]() -> std::atomic<std::time_t> & {
- std::lock_guard<std::mutex> Lock(Entries.Mutex);
- auto &Entry = Entries.Map[Filename];
- if (!Entry)
- Entry = std::make_unique<ModuleCacheEntry>();
- return Entry->Timestamp;
- }();
-
- return Timestamp.load();
+ return getOrCreateEntry(Filename).Timestamp.load();
}
void updateModuleTimestamp(StringRef Filename) override {
// Note: This essentially replaces FS contention with mutex contention.
- auto &Timestamp = [&]() -> std::atomic<std::time_t> & {
- std::lock_guard<std::mutex> Lock(Entries.Mutex);
- auto &Entry = Entries.Map[Filename];
- if (!Entry)
- Entry = std::make_unique<ModuleCacheEntry>();
- return Entry->Timestamp;
- }();
-
- Timestamp.store(llvm::sys::toTimeT(std::chrono::system_clock::now()));
+ getOrCreateEntry(Filename).Timestamp.store(
+ llvm::sys::toTimeT(std::chrono::system_clock::now()));
}
void maybePrune(StringRef Path, time_t PruneInterval,
@@ -134,12 +137,13 @@ class InProcessModuleCache : public ModuleCache {
std::error_code write(StringRef Path, llvm::MemoryBufferRef Buffer,
off_t &Size, time_t &ModTime) override {
- // This is a compiler-internal input/output, let's bypass the sandbox.
- auto BypassSandbox = llvm::sys::sandbox::scopedDisable();
-
- // FIXME: This could use an in-memory cache to avoid IO, and only write to
- // disk at the end of the scan.
- return writeImpl(Path, Buffer, Size, ModTime);
+ ModuleCacheEntry &Entry = getOrCreateEntry(Path);
+ std::lock_guard<std::mutex> Lock(Entry.Mutex);
+ Entry.Buffer =
+ llvm::MemoryBuffer::getMemBufferCopy(Buffer.getBuffer(), Path);
+ Entry.ModTime = llvm::sys::toTimeT(std::chrono::system_clock::now());
+ Entry.NeedsFlush = true;
+ return {};
}
Expected<std::unique_ptr<llvm::MemoryBuffer>>
@@ -147,9 +151,21 @@ class InProcessModuleCache : public ModuleCache {
// This is a compiler-internal input/output, let's bypass the sandbox.
auto BypassSandbox = llvm::sys::sandbox::scopedDisable();
- // FIXME: This only needs to go to disk once per build, not in every
- // compilation. Introduce in-memory cache.
- return readImpl(FileName, Size, ModTime);
+ ModuleCacheEntry &Entry = getOrCreateEntry(FileName);
+ std::lock_guard<std::mutex> Lock(Entry.Mutex);
+ if (!Entry.Buffer) {
+ off_t ReadSize;
+ time_t ReadModTime;
+ auto ReadBuffer = readImpl(FileName, ReadSize, ReadModTime);
+ if (!ReadBuffer)
+ return ReadBuffer.takeError();
+ Entry.Buffer = std::move(*ReadBuffer);
+ Entry.ModTime = ReadModTime;
+ Entry.NeedsFlush = false;
+ }
+ Size = Entry.Buffer->getBufferSize();
+ ModTime = Entry.ModTime;
+ return llvm::MemoryBuffer::getMemBuffer(*Entry.Buffer);
}
};
} // namespace
diff --git a/clang/lib/Serialization/ModuleCache.cpp b/clang/lib/Serialization/ModuleCache.cpp
index efbaf9765546f..a61724819530a 100644
--- a/clang/lib/Serialization/ModuleCache.cpp
+++ b/clang/lib/Serialization/ModuleCache.cpp
@@ -24,9 +24,22 @@ const ModuleCacheDirectory *ModuleCache::getDirectoryPtr(StringRef Path) {
if (!ByNameIt->second) {
// This is a compiler-internal input/output, let's bypass the sandbox.
auto BypassSandbox = llvm::sys::sandbox::scopedDisable();
+
+ // If we cannot get status of the module cache directory, try if trying to
+ // create it helps.
llvm::sys::fs::file_status Status;
- if (llvm::sys::fs::status(Path, Status))
- return nullptr;
+ if (std::error_code EC = llvm::sys::fs::status(Path, Status)) {
+ // Unless the status failed because the directory does not exist yet.
+ if (EC != std::errc::no_such_file_or_directory)
+ return nullptr;
+ // If we're unable to create the directory.
+ if (llvm::sys::fs::create_directories(Path))
+ return nullptr;
+ // If we're unable to stat the newly created directory.
+ if (llvm::sys::fs::status(Path, Status))
+ return nullptr;
+ }
+
llvm::sys::fs::UniqueID UID = Status.getUniqueID();
auto [ByUIDIt, ByUIDInserted] = ByUID.insert({UID, nullptr});
if (!ByUIDIt->second)
diff --git a/clang/tools/clang-scan-deps/ClangScanDeps.cpp b/clang/tools/clang-scan-deps/ClangScanDeps.cpp
index fd3d70f9498ee..54343df29efe1 100644
--- a/clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ b/clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -1159,6 +1159,8 @@ int clang_scan_deps_main(int argc, char **argv, const llvm::ToolContext &) {
Pool.wait();
}
+ Service.getModuleCacheEntries().flush();
+
T.stopTimer();
if (Verbose)
|
🐧 Linux x64 Test Results
✅ The build succeeded and all tests passed. |
a5629bf to
ba2a4f7
Compare
We did not initialize the out parameters in llvm#192347, causing the "sanitizer-x86_64-linux-fast" bot to complain with: ``` SUMMARY: MemorySanitizer: use-of-uninitialized-value /home/b/sanitizer-x86_64-linux-fast/build/llvm-project/clang/lib/Frontend/CompilerInstance.cpp:1525:63 in compileModuleImpl(clang::CompilerInstance&, clang::SourceLocation, clang::SourceLocation, clang::Module*, clang::ModuleFileName) Exiting ==clang==3084515==WARNING: MemorySanitizer: use-of-uninitialized-value #0 0x586360f7a604 in compileModuleImpl(clang::CompilerInstance&, clang::SourceLocation, clang::SourceLocation, clang::Module*, clang::ModuleFileName) /home/b/sanitizer-x86_64-linux-fast/build/llvm-project/clang/lib/Frontend/CompilerInstance.cpp:1525:63 ... ``` This PR should fix that.
We did not initialize the out parameters in #192347, causing the "sanitizer-x86_64-linux-fast" bot to complain with: ``` SUMMARY: MemorySanitizer: use-of-uninitialized-value /home/b/sanitizer-x86_64-linux-fast/build/llvm-project/clang/lib/Frontend/CompilerInstance.cpp:1525:63 in compileModuleImpl(clang::CompilerInstance&, clang::SourceLocation, clang::SourceLocation, clang::Module*, clang::ModuleFileName) Exiting ==clang==3084515==WARNING: MemorySanitizer: use-of-uninitialized-value #0 0x586360f7a604 in compileModuleImpl(clang::CompilerInstance&, clang::SourceLocation, clang::SourceLocation, clang::Module*, clang::ModuleFileName) /home/b/sanitizer-x86_64-linux-fast/build/llvm-project/clang/lib/Frontend/CompilerInstance.cpp:1525:63 #1 <...> ``` This PR should fix that.
|
One of our internal builds has started failing its tests after this commit. Unfortunately it is proving difficult to reproduce in a form that we can work out what is going wrong. It fails an assertion, on a couple of clang tests (details below), but only one one of our CI machines. All attempts to reproduce (using the same docker container) have so far failed. The main difference in environment is that CI machine has more threads (96 vs 64). I don't see this failing on any other buildbot, there's one semi-related mention of a race condition in #194475 (comment) but that's for a different test to what we're seeing problems in. Would you have any advice for us to try and narrow down what the problem is so we can try and make a consistent reproducer? We originally thought this might be multiple tests writing to the same file, but it doesn't look like this is the case. The CMake command for the toolchain is: Which is building a cross compiler running on x86_64 targeting aarch64-linux-gnu. The build is in an Ubuntu 24.04 docker container using the distros GCC 13.3.0 as the compiler. The test failures specific to building 32eb90b are: these trip the same assertion failure in clang-scan-deps: I've attached an extract of the log including the backtrace (sadly from a release build) as it is a bit too large to quote here. |
|
Asking an LLM to find out when the Assertion In Later there is call to getMemBuffer() (https://github.com/llvm/llvm-project/blob/main/llvm/include/llvm/Support/MemoryBuffer.h#L139) which has a default of /* RequiresNullTerminator */ true https://github.com/llvm/llvm-project/blob/main/clang/lib/DependencyScanning/InProcessModuleCache.cpp#L181 So there is a potential mismatch of RequiresNullTerminator. That sounds plausible although I'm not sure why this only triggers on one CI machine and not locally. Apologies if this is just noise. |
|
Thanks for the investigation, that sounds plausible. Would you mind posting a patch that passes |
With this PR, module cache PCMs are kept in-memory and can be flushed to disk at the end of the build if desired. Not going to disk at all speeds up dependency scans by around 4.7% by avoiding contention in the kernel.
We did not initialize the out parameters in llvm#192347, causing the "sanitizer-x86_64-linux-fast" bot to complain with: ``` SUMMARY: MemorySanitizer: use-of-uninitialized-value /home/b/sanitizer-x86_64-linux-fast/build/llvm-project/clang/lib/Frontend/CompilerInstance.cpp:1525:63 in compileModuleImpl(clang::CompilerInstance&, clang::SourceLocation, clang::SourceLocation, clang::Module*, clang::ModuleFileName) Exiting ==clang==3084515==WARNING: MemorySanitizer: use-of-uninitialized-value #0 0x586360f7a604 in compileModuleImpl(clang::CompilerInstance&, clang::SourceLocation, clang::SourceLocation, clang::Module*, clang::ModuleFileName) /home/b/sanitizer-x86_64-linux-fast/build/llvm-project/clang/lib/Frontend/CompilerInstance.cpp:1525:63 llvm#1 <...> ``` This PR should fix that.
Will do, I'm working on how to inject a local patch into our CI-system to check, will post a PR if it works. |
Apologies for the delay, it took a while to work out how to get the CI job to accept a local patch. I created a Draft PR #194867 and have applied that patch. It does fix our local problem. I can make that Draft PR into a full one if it is sufficient? What I've not been able to do is find a test case that can fail on my local machine yet. The two working theories I've got at the moment:
|
With this PR, module cache PCMs are kept in-memory and can be flushed to disk at the end of the build if desired. Not going to disk at all speeds up dependency scans by around 4.7% by avoiding contention in the kernel.
We did not initialize the out parameters in llvm#192347, causing the "sanitizer-x86_64-linux-fast" bot to complain with: ``` SUMMARY: MemorySanitizer: use-of-uninitialized-value /home/b/sanitizer-x86_64-linux-fast/build/llvm-project/clang/lib/Frontend/CompilerInstance.cpp:1525:63 in compileModuleImpl(clang::CompilerInstance&, clang::SourceLocation, clang::SourceLocation, clang::Module*, clang::ModuleFileName) Exiting ==clang==3084515==WARNING: MemorySanitizer: use-of-uninitialized-value #0 0x586360f7a604 in compileModuleImpl(clang::CompilerInstance&, clang::SourceLocation, clang::SourceLocation, clang::Module*, clang::ModuleFileName) /home/b/sanitizer-x86_64-linux-fast/build/llvm-project/clang/lib/Frontend/CompilerInstance.cpp:1525:63 llvm#1 <...> ``` This PR should fix that.
|
I've raised a full PR #195072 and have added you as a reviewer. |
| void ModuleCacheEntries::flush() { | ||
| auto BypassSandbox = llvm::sys::sandbox::scopedDisable(); | ||
| for (auto &[Path, Entry] : Map) { | ||
| if (Entry->State == ModuleCacheEntry::S_Written) { |
There was a problem hiding this comment.
There should be a comment that we are not locking the Mutex b/c this is only called from the destructor, probably a comment on top of the function. Something like "This should only be called from the destructor because we are not taking the lock on the mutex below"
With this PR, module cache PCMs are kept in-memory and can be flushed to disk at the end of the build if desired. Not going to disk at all speeds up dependency scans by around 4.7% by avoiding contention in the kernel.