[PATCH] D115043: [clang][deps] Use MemoryBuffer in minimizing FS

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 created this revision.
jansvoboda11 added reviewers: Bigcheese, dexonsmith, arphaman.
jansvoboda11 requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115043

Files:
  clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
  clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
  llvm/include/llvm/Support/SmallVectorMemoryBuffer.h


Index: llvm/include/llvm/Support/SmallVectorMemoryBuffer.h
===================================================================
--- llvm/include/llvm/Support/SmallVectorMemoryBuffer.h
+++ llvm/include/llvm/Support/SmallVectorMemoryBuffer.h
@@ -14,6 +14,7 @@
 #ifndef LLVM_SUPPORT_SMALLVECTORMEMORYBUFFER_H
 #define LLVM_SUPPORT_SMALLVECTORMEMORYBUFFER_H
 
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/raw_ostream.h"
@@ -48,6 +49,16 @@
     init(this->SV.begin(), this->SV.end(), false);
   }
 
+  /// Construct a SmallVectorMemoryBuffer from the given SmallVector r-value,
+  /// and invoke the given function right after the move.
+  SmallVectorMemoryBuffer(
+      SmallVectorImpl<char> &&SV,
+      llvm::function_ref<void(SmallVectorImpl<char> &)> AfterMove)
+      : SV(std::move(SV)), BufferName("<in-memory object>") {
+    AfterMove(this->SV);
+    init(this->SV.begin(), this->SV.end(), false);
+  }
+
   // Key function.
   ~SmallVectorMemoryBuffer() override;
 
Index: clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
===================================================================
--- clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
+++ 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 @@
     // 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;
   }
 
@@ -65,10 +62,12 @@
   // 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), [](SmallVectorImpl<char> &SV) {
+        // Now make the null terminator implicit again, so that Clang's lexer
+        // can find it right where the buffer ends.
+        SV.pop_back();
+      });
 
   // Compute the skipped PP ranges that speedup skipping over inactive
   // preprocessor blocks.
Index: clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
===================================================================
--- clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
+++ clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
@@ -65,7 +65,7 @@
       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 +94,7 @@
 
 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;
 };
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D115043.391643.patch
Type: text/x-patch
Size: 4028 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20211203/51cf3eb4/attachment.bin>