[llvm] 429088b - Support: Extract fs::resize_file_before_mapping_readwrite from FileOutputBuffer

[Duncan P. N. Exon Smith via llvm-commits]

Author: Duncan P. N. Exon Smith
Date: 2021-04-08T16:26:35-07:00
New Revision: 429088b9e214b2f39f6063a6b3639560fc7db47b

URL: https://github.com/llvm/llvm-project/commit/429088b9e214b2f39f6063a6b3639560fc7db47b
DIFF: https://github.com/llvm/llvm-project/commit/429088b9e214b2f39f6063a6b3639560fc7db47b.diff

LOG: Support: Extract fs::resize_file_before_mapping_readwrite from FileOutputBuffer

Add a variant of `fs::resize_file` for use immediately before opening a
file with `mapped_file_region::readwrite`. On Windows, `_chsize`
(`ftruncate`) is slow, but `CreateFileMapping` (`mmap`) automatically
extends the file so the call to `fs::resize_file` can be skipped.

This optimization was added to `FileOutputBuffer` in
da9bc2e56d5a5c6332a9def1a0065eb399182b93; this commit just extracts the
logic out and adds a unit test.

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

Added: 
    

Modified: 
    llvm/include/llvm/Support/FileSystem.h
    llvm/lib/Support/FileOutputBuffer.cpp
    llvm/unittests/Support/Path.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/Support/FileSystem.h b/llvm/include/llvm/Support/FileSystem.h
index 2ed7d87690cbb..da3627721ebd1 100644
--- a/llvm/include/llvm/Support/FileSystem.h
+++ b/llvm/include/llvm/Support/FileSystem.h
@@ -411,6 +411,21 @@ std::error_code copy_file(const Twine &From, int ToFD);
 ///          platform-specific error_code.
 std::error_code resize_file(int FD, uint64_t Size);
 
+/// Resize \p FD to \p Size before mapping \a mapped_file_region::readwrite. On
+/// non-Windows, this calls \a resize_file(). On Windows, this is a no-op,
+/// since the subsequent mapping (via \c CreateFileMapping) automatically
+/// extends the file.
+inline std::error_code resize_file_before_mapping_readwrite(int FD,
+                                                            uint64_t Size) {
+#ifdef _WIN32
+  (void)FD;
+  (void)Size;
+  return std::error_code();
+#else
+  return resize_file(FD, Size);
+#endif
+}
+
 /// Compute an MD5 hash of a file's contents.
 ///
 /// @param FD Input file descriptor.

diff  --git a/llvm/lib/Support/FileOutputBuffer.cpp b/llvm/lib/Support/FileOutputBuffer.cpp
index 3342682270dcd..6d2125802f257 100644
--- a/llvm/lib/Support/FileOutputBuffer.cpp
+++ b/llvm/lib/Support/FileOutputBuffer.cpp
@@ -132,17 +132,10 @@ createOnDiskBuffer(StringRef Path, size_t Size, unsigned Mode) {
     return FileOrErr.takeError();
   fs::TempFile File = std::move(*FileOrErr);
 
-#ifndef _WIN32
-  // On Windows, CreateFileMapping (the mmap function on Windows)
-  // automatically extends the underlying file. We don't need to
-  // extend the file beforehand. _chsize (ftruncate on Windows) is
-  // pretty slow just like it writes specified amount of bytes,
-  // so we should avoid calling that function.
-  if (auto EC = fs::resize_file(File.FD, Size)) {
+  if (auto EC = fs::resize_file_before_mapping_readwrite(File.FD, Size)) {
     consumeError(File.discard());
     return errorCodeToError(EC);
   }
-#endif
 
   // Mmap it.
   std::error_code EC;

diff  --git a/llvm/unittests/Support/Path.cpp b/llvm/unittests/Support/Path.cpp
index 35b41092fddaa..6615d2f2806f3 100644
--- a/llvm/unittests/Support/Path.cpp
+++ b/llvm/unittests/Support/Path.cpp
@@ -1289,6 +1289,31 @@ TEST_F(FileSystemTest, Resize) {
   ASSERT_NO_ERROR(fs::remove(TempPath));
 }
 
+TEST_F(FileSystemTest, ResizeBeforeMapping) {
+  // Create a temp file.
+  int FD;
+  SmallString<64> TempPath;
+  ASSERT_NO_ERROR(fs::createTemporaryFile("prefix", "temp", FD, TempPath));
+  ASSERT_NO_ERROR(fs::resize_file_before_mapping_readwrite(FD, 123));
+
+  // Map in temp file. On Windows, fs::resize_file_before_mapping_readwrite is
+  // a no-op and the mapping itself will resize the file.
+  std::error_code EC;
+  {
+    fs::mapped_file_region mfr(fs::convertFDToNativeFile(FD),
+                               fs::mapped_file_region::readwrite, 123, 0, EC);
+    ASSERT_NO_ERROR(EC);
+    // Unmap temp file
+  }
+
+  // Check the size.
+  fs::file_status Status;
+  ASSERT_NO_ERROR(fs::status(FD, Status));
+  ASSERT_EQ(Status.getSize(), 123U);
+  ::close(FD);
+  ASSERT_NO_ERROR(fs::remove(TempPath));
+}
+
 TEST_F(FileSystemTest, MD5) {
   int FD;
   SmallString<64> TempPath;
@@ -1310,7 +1335,8 @@ TEST_F(FileSystemTest, FileMapping) {
   ASSERT_NO_ERROR(
       fs::createTemporaryFile("prefix", "temp", FileDescriptor, TempPath));
   unsigned Size = 4096;
-  ASSERT_NO_ERROR(fs::resize_file(FileDescriptor, Size));
+  ASSERT_NO_ERROR(
+      fs::resize_file_before_mapping_readwrite(FileDescriptor, Size));
 
   // Map in temp file and add some content
   std::error_code EC;