[LLD][ELF] Support --[no-]mmap-output-file with F_no_mmap

Summary:
Add a flag `F_no_mmap` to `FileOutputBuffer` to support
`--[no-]mmap-output-file` in ELF LLD. LLD currently explicitly ignores
this flag for compatibility with GNU ld and gold.

We need this flag to speed up link time for large binaries in certain
scenarios. When we link some of our larger binaries we find that LLD
takes 50+ GB of memory, which causes memory pressure. The memory
pressure causes the VM to flush dirty pages of the output file to disk.
This is normally okay, since we should be flushing cold pages. However,
when using BtrFS with compression we need to write 128KB at a time when
we flush a page. If any page in that 128KB block is written again, then
it must be flushed a second time, and so on. Since LLD doesn't write
sequentially this causes write amplification. The same 128KB block will
end up being flushed multiple times, causing the linker to many times
more IO than necessary. We've observed 3-5x faster builds with
-no-mmap-output-file when we hit this scenario.

The bad scenario only applies to compressed filesystems, which group
together multiple pages into a single compressed block. I've tested
BtrFS, but the problem will be present for any compressed filesystem
on Linux, since it is caused by the VM.

Silently ignoring --no-mmap-output-file caused a silent regression when
we switched from gold to lld. We pass --no-mmap-output-file to fix this
edge case, but since lld silently ignored the flag we didn't realize it
wasn't being respected.

Benchmark building a 9 GB binary that exposes this edge case. I linked 3
times with --mmap-output-file and 3 times with --no-mmap-output-file and
took the average. The machine has 24 cores @ 2.4 GHz, 112 GB of RAM,
BtrFS mounted with -compress-force=zstd, and an 80% full disk.

| Mode    | Time  |
|---------|-------|
| mmap    | 894 s |
| no mmap | 126 s |

When compression is disabled, BtrFS performs just as well with and
without mmap on this benchmark.

I was unable to reproduce the regression with any binaries in
lld-speed-test.

Reviewed By: ruiu, MaskRay

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

diff --git a/lld/ELF/Config.h b/lld/ELF/Config.h
index af27f8b..ddc195b 100644 (file)
--- a/lld/ELF/Config.h
+++ b/lld/ELF/Config.h
@@ -168,6 +168,7 @@ struct Configuration {
   bool ltoNewPassManager;
   bool mergeArmExidx;
   bool mipsN32Abi = false;
+  bool mmapOutputFile;
   bool nmagic;
   bool noinhibitExec;
   bool nostdlib;

diff --git a/lld/ELF/Driver.cpp b/lld/ELF/Driver.cpp
index 35d092e..1b1a50b 100644 (file)
--- a/lld/ELF/Driver.cpp
+++ b/lld/ELF/Driver.cpp
@@ -900,6 +900,8 @@ static void readConfigs(opt::InputArgList &args) {
   config->mipsGotSize = args::getInteger(args, OPT_mips_got_size, 0xfff0);
   config->mergeArmExidx =
       args.hasFlag(OPT_merge_exidx_entries, OPT_no_merge_exidx_entries, true);
+  config->mmapOutputFile =
+      args.hasFlag(OPT_mmap_output_file, OPT_no_mmap_output_file, true);
   config->nmagic = args.hasFlag(OPT_nmagic, OPT_no_nmagic, false);
   config->noinhibitExec = args.hasArg(OPT_noinhibit_exec);
   config->nostdlib = args.hasArg(OPT_nostdlib);

diff --git a/lld/ELF/Options.td b/lld/ELF/Options.td
index 358d01c..c6773d0 100644 (file)
--- a/lld/ELF/Options.td
+++ b/lld/ELF/Options.td
@@ -232,6 +232,10 @@ defm merge_exidx_entries: B<"merge-exidx-entries",
     "Enable merging .ARM.exidx entries (default)",
     "Disable merging .ARM.exidx entries">;
 
+defm mmap_output_file: B<"mmap-output-file",
+    "Mmap the output file for writing (default)",
+    "Do not mmap the output file for writing">;
+
 def nmagic: F<"nmagic">, MetaVarName<"<magic>">,
   HelpText<"Do not page align sections, link against static libraries.">;
 
@@ -566,7 +570,6 @@ def: F<"no-add-needed">;
 def: F<"no-copy-dt-needed-entries">;
 def: F<"no-ctors-in-init-array">;
 def: F<"no-keep-memory">;
-def: F<"no-mmap-output-file">;
 def: F<"no-pipeline-knowledge">;
 def: F<"no-warn-mismatch">;
 def: Flag<["-"], "p">;

diff --git a/lld/ELF/Writer.cpp b/lld/ELF/Writer.cpp
index 8e25357..3de1230 100644 (file)
--- a/lld/ELF/Writer.cpp
+++ b/lld/ELF/Writer.cpp
@@ -2595,7 +2595,9 @@ template <class ELFT> void Writer<ELFT>::openFile() {
   unlinkAsync(config->outputFile);
   unsigned flags = 0;
   if (!config->relocatable)
-    flags = FileOutputBuffer::F_executable;
+    flags |= FileOutputBuffer::F_executable;
+  if (!config->mmapOutputFile)
+    flags |= FileOutputBuffer::F_no_mmap;
   Expected<std::unique_ptr<FileOutputBuffer>> bufferOrErr =
       FileOutputBuffer::create(config->outputFile, fileSize, flags);
 

diff --git a/lld/test/ELF/silent-ignore.test b/lld/test/ELF/silent-ignore.test
index 3371ed6..600c9f8 100644 (file)
--- a/lld/test/ELF/silent-ignore.test
+++ b/lld/test/ELF/silent-ignore.test
@@ -6,7 +6,6 @@ RUN:   -no-add-needed \
 RUN:   -no-copy-dt-needed-entries \
 RUN:   -no-ctors-in-init-array \
 RUN:   -no-keep-memory \
-RUN:   -no-mmap-output-file \
 RUN:   -no-pipeline-knowledge \
 RUN:   -no-warn-mismatch \
 RUN:   -p \

diff --git a/llvm/include/llvm/Support/FileOutputBuffer.h b/llvm/include/llvm/Support/FileOutputBuffer.h
index 999f551..bdc1425 100644 (file)
--- a/llvm/include/llvm/Support/FileOutputBuffer.h
+++ b/llvm/include/llvm/Support/FileOutputBuffer.h
@@ -32,6 +32,10 @@ public:
   enum {
     /// set the 'x' bit on the resulting file
     F_executable = 1,
+
+    /// Don't use mmap and instead write an in-memory buffer to a file when this
+    /// buffer is closed.
+    F_no_mmap = 2,
   };
 
   /// Factory method to create an OutputBuffer object which manages a read/write

diff --git a/llvm/lib/Support/FileOutputBuffer.cpp b/llvm/lib/Support/FileOutputBuffer.cpp
index 024dd3e..0a5306f 100644 (file)
--- a/llvm/lib/Support/FileOutputBuffer.cpp
+++ b/llvm/lib/Support/FileOutputBuffer.cpp
@@ -189,7 +189,10 @@ FileOutputBuffer::create(StringRef Path, size_t Size, unsigned Flags) {
   case fs::file_type::regular_file:
   case fs::file_type::file_not_found:
   case fs::file_type::status_error:
-    return createOnDiskBuffer(Path, Size, Mode);
+    if (Flags & F_no_mmap)
+      return createInMemoryBuffer(Path, Size, Mode);
+    else
+      return createOnDiskBuffer(Path, Size, Mode);
   default:
     return createInMemoryBuffer(Path, Size, Mode);
   }

diff --git a/llvm/unittests/Support/FileOutputBufferTest.cpp b/llvm/unittests/Support/FileOutputBufferTest.cpp
index 8afc212..6b6196f 100644 (file)
--- a/llvm/unittests/Support/FileOutputBufferTest.cpp
+++ b/llvm/unittests/Support/FileOutputBufferTest.cpp
@@ -118,6 +118,28 @@ TEST(FileOutputBuffer, Test) {
   EXPECT_TRUE(IsExecutable);
   ASSERT_NO_ERROR(fs::remove(File4.str()));
 
+  // TEST 5: In-memory buffer works as expected.
+  SmallString<128> File5(TestDirectory);
+  File5.append("/file5");
+  {
+    Expected<std::unique_ptr<FileOutputBuffer>> BufferOrErr =
+        FileOutputBuffer::create(File5, 8000, FileOutputBuffer::F_no_mmap);
+    ASSERT_NO_ERROR(errorToErrorCode(BufferOrErr.takeError()));
+    std::unique_ptr<FileOutputBuffer> &Buffer = *BufferOrErr;
+    // Start buffer with special header.
+    memcpy(Buffer->getBufferStart(), "AABBCCDDEEFFGGHHIIJJ", 20);
+    ASSERT_NO_ERROR(errorToErrorCode(Buffer->commit()));
+    // Write to end of buffer to verify it is writable.
+    memcpy(Buffer->getBufferEnd() - 20, "AABBCCDDEEFFGGHHIIJJ", 20);
+    // Commit buffer.
+    ASSERT_NO_ERROR(errorToErrorCode(Buffer->commit()));
+  }
+
+  // Verify file is correct size.
+  uint64_t File5Size;
+  ASSERT_NO_ERROR(fs::file_size(Twine(File5), File5Size));
+  ASSERT_EQ(File5Size, 8000ULL);
+  ASSERT_NO_ERROR(fs::remove(File5.str()));
   // Clean up.
   ASSERT_NO_ERROR(fs::remove(TestDirectory.str()));
 }