diff options
author | Adrian Prantl <aprantl@apple.com> | 2018-08-20 17:10:27 +0000 |
---|---|---|
committer | Adrian Prantl <aprantl@apple.com> | 2018-08-20 17:10:27 +0000 |
commit | 6f4f824d5674b1ab5276cd5e3cd93bb7ede784ca (patch) | |
tree | b1845bf0ebc2280d4363d286fa6f846d179fe040 /lib/Serialization | |
parent | ae679b05d34a1503557bd00c1d99d327b6f413e9 (diff) | |
download | clang-6f4f824d5674b1ab5276cd5e3cd93bb7ede784ca.tar.gz |
Close FileEntries of cached files in ModuleManager::addModule().
While investigating why LLDB (which can build hundreds of clang
modules during one debug session) was getting "too many open files"
errors, I found that most of them are .pcm files that are kept open by
ModuleManager. Pretty much all of the open file dscriptors are
FileEntries that are refering to `.pcm` files for which a buffer
already exists in a CompilerInstance's PCMCache.
Before PCMCache was added it was necessary to hold on to open file
descriptors to ensure that all ModuleManagers using the same
FileManager read the a consistent version of a given `.pcm` file on
disk, even when a concurrent clang process overwrites the file halfway
through. The PCMCache makes this practice unnecessary, since it caches
the entire contents of a `.pcm` file, while the FileManager caches all
the stat() information.
This patch adds a call to FileEntry::closeFile() to the path where a
Buffer has already been created. This is necessary because even for a
freshly written `.pcm` file the file is stat()ed once immediately
after writing to generate a FileEntry in the FileManager. Because a
freshly-generated file's contents is stored in the PCMCache, it is
fine to close the file immediately thereafter. The second change this
patch makes is to set the `ShouldClose` flag to true when reading a
`.pcm` file into the PCMCache for the first time.
[For reference, in 1 Clang instance there is
- 1 FileManager and
- n ModuleManagers with
- n PCMCaches.]
rdar://problem/40906753
Differential Revision: https://reviews.llvm.org/D50870
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@340188 91177308-0d34-0410-b5e6-96231b3b80d8
Diffstat (limited to 'lib/Serialization')
-rw-r--r-- | lib/Serialization/ModuleManager.cpp | 13 |
1 files changed, 8 insertions, 5 deletions
diff --git a/lib/Serialization/ModuleManager.cpp b/lib/Serialization/ModuleManager.cpp index 57ebaca10c..fccfa88ab9 100644 --- a/lib/Serialization/ModuleManager.cpp +++ b/lib/Serialization/ModuleManager.cpp @@ -161,21 +161,24 @@ ModuleManager::addModule(StringRef FileName, ModuleKind Type, if (std::unique_ptr<llvm::MemoryBuffer> Buffer = lookupBuffer(FileName)) { // The buffer was already provided for us. NewModule->Buffer = &PCMCache->addBuffer(FileName, std::move(Buffer)); + // Since the cached buffer is reused, it is safe to close the file + // descriptor that was opened while stat()ing the PCM in + // lookupModuleFile() above, it won't be needed any longer. + Entry->closeFile(); } else if (llvm::MemoryBuffer *Buffer = PCMCache->lookupBuffer(FileName)) { NewModule->Buffer = Buffer; + // As above, the file descriptor is no longer needed. + Entry->closeFile(); } else { // Open the AST file. llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> Buf((std::error_code())); if (FileName == "-") { Buf = llvm::MemoryBuffer::getSTDIN(); } else { - // Leave the FileEntry open so if it gets read again by another - // ModuleManager it must be the same underlying file. - // FIXME: Because FileManager::getFile() doesn't guarantee that it will - // give us an open file, this may not be 100% reliable. + // Get a buffer of the file and close the file descriptor when done. Buf = FileMgr.getBufferForFile(NewModule->File, /*IsVolatile=*/false, - /*ShouldClose=*/false); + /*ShouldClose=*/true); } if (!Buf) { |