[clang] 13a351e - [clang][deps] Use MemoryBuffer in minimizing FS

[Jan Svoboda via cfe-commits]

Author: Jan Svoboda
Date: 2021-12-09T11:32:13+01:00
New Revision: 13a351e862ba3f443b31fac57863625a5c43e43b

URL: https://github.com/llvm/llvm-project/commit/13a351e862ba3f443b31fac57863625a5c43e43b
DIFF: https://github.com/llvm/llvm-project/commit/13a351e862ba3f443b31fac57863625a5c43e43b.diff

LOG: [clang][deps] Use MemoryBuffer in minimizing FS

This patch avoids unnecessarily copying contents of `mmap`-ed files into `CachedFileSystemEntry` by storing `MemoryBuffer` instead. The change leads to ~50% reduction of peak memory footprint when scanning LLVM+Clang via `clang-scan-deps`.

Depends on D115331.

Reviewed By: dexonsmith

Differential Revision: https://reviews.llvm.org/D115043

Added: 
    

Modified: 
    clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
    clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
index af02fa2e7e876..25a7d57583691 100644
--- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
+++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
@@ -43,9 +43,7 @@ class CachedFileSystemEntry {
   ///
   /// The filesystem opens the file even for `stat` calls open to avoid the
   /// issues with stat + open of minimized files that might lead to a
-  /// mismatching size of the file. If file is not minimized, the full file is
-  /// read and copied into memory to ensure that it's not memory mapped to avoid
-  /// running out of file descriptors.
+  /// mismatching size of the file.
   static CachedFileSystemEntry createFileEntry(StringRef Filename,
                                                llvm::vfs::FileSystem &FS,
                                                bool Minimize = true);
@@ -65,7 +63,7 @@ class CachedFileSystemEntry {
       return MaybeStat.getError();
     assert(!MaybeStat->isDirectory() && "not a file");
     assert(isValid() && "not initialized");
-    return Contents.str();
+    return Contents->getBuffer();
   }
 
   /// \returns The error or the status of the entry.
@@ -94,11 +92,7 @@ class CachedFileSystemEntry {
 
 private:
   llvm::ErrorOr<llvm::vfs::Status> MaybeStat;
-  // Store the contents in a small string to allow a
-  // move from the small string for the minimized contents.
-  // Note: small size of 1 allows us to store an empty string with an implicit
-  // null terminator without any allocations.
-  llvm::SmallString<1> Contents;
+  std::unique_ptr<llvm::MemoryBuffer> Contents;
   PreprocessorSkippedRangeMapping PPSkippedRangeMapping;
 };
 

diff  --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
index 8a85f5971f4b2..367989f436d37 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
@@ -9,6 +9,7 @@
 #include "clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h"
 #include "clang/Lex/DependencyDirectivesSourceMinimizer.h"
 #include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/SmallVectorMemoryBuffer.h"
 #include "llvm/Support/Threading.h"
 
 using namespace clang;
@@ -43,11 +44,7 @@ CachedFileSystemEntry CachedFileSystemEntry::createFileEntry(
     // FIXME: Propage the diagnostic if desired by the client.
     CachedFileSystemEntry Result;
     Result.MaybeStat = std::move(*Stat);
-    Result.Contents.reserve(Buffer->getBufferSize() + 1);
-    Result.Contents.append(Buffer->getBufferStart(), Buffer->getBufferEnd());
-    // Implicitly null terminate the contents for Clang's lexer.
-    Result.Contents.push_back('\0');
-    Result.Contents.pop_back();
+    Result.Contents = std::move(*MaybeBuffer);
     return Result;
   }
 
@@ -60,15 +57,8 @@ CachedFileSystemEntry CachedFileSystemEntry::createFileEntry(
   // The contents produced by the minimizer must be null terminated.
   assert(MinimizedFileContents.data()[MinimizedFileContents.size()] == '\0' &&
          "not null terminated contents");
-  // Even though there's an implicit null terminator in the minimized contents,
-  // we want to temporarily make it explicit. This will ensure that the
-  // std::move will preserve it even if it needs to do a copy if the
-  // SmallString still has the small capacity.
-  MinimizedFileContents.push_back('\0');
-  Result.Contents = std::move(MinimizedFileContents);
-  // Now make the null terminator implicit again, so that Clang's lexer can find
-  // it right where the buffer ends.
-  Result.Contents.pop_back();
+  Result.Contents = std::make_unique<llvm::SmallVectorMemoryBuffer>(
+      std::move(MinimizedFileContents));
 
   // Compute the skipped PP ranges that speedup skipping over inactive
   // preprocessor blocks.