[lld] 6814232 - [LLD][ELF] Support --[no-]mmap-output-file with F_no_mmap
Fangrui Song via llvm-commits
llvm-commits at lists.llvm.org
Tue Oct 29 15:49:38 PDT 2019
Author: Nick Terrell
Date: 2019-10-29T15:49:08-07:00
New Revision: 68142324290f2932df0e271747cdccc371d6dded
URL: https://github.com/llvm/llvm-project/commit/68142324290f2932df0e271747cdccc371d6dded
DIFF: https://github.com/llvm/llvm-project/commit/68142324290f2932df0e271747cdccc371d6dded.diff
LOG: [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
Added:
Modified:
lld/ELF/Config.h
lld/ELF/Driver.cpp
lld/ELF/Options.td
lld/ELF/Writer.cpp
lld/test/ELF/silent-ignore.test
llvm/include/llvm/Support/FileOutputBuffer.h
llvm/lib/Support/FileOutputBuffer.cpp
llvm/unittests/Support/FileOutputBufferTest.cpp
Removed:
################################################################################
diff --git a/lld/ELF/Config.h b/lld/ELF/Config.h
index af27f8b13c76..ddc195b8e4f8 100644
--- 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 35d092ea7658..1b1a50bf6d1c 100644
--- 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 358d01cefce3..c6773d0122e0 100644
--- 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 8e253577385e..3de1230150d6 100644
--- 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 3371ed6a2d91..600c9f86b18a 100644
--- 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 999f551ebf2d..bdc1425d4361 100644
--- a/llvm/include/llvm/Support/FileOutputBuffer.h
+++ b/llvm/include/llvm/Support/FileOutputBuffer.h
@@ -32,6 +32,10 @@ class FileOutputBuffer {
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 024dd3e57a40..0a5306f684d4 100644
--- 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 8afc2125ef46..6b6196f97164 100644
--- 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()));
}
More information about the llvm-commits
mailing list