Skip to content

Commit 6814232

Browse files
terrellnMaskRay
authored andcommitted
[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
1 parent dbcb690 commit 6814232

File tree

8 files changed

+40
-4
lines changed

8 files changed

+40
-4
lines changed

lld/ELF/Config.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,7 @@ struct Configuration {
168168
bool ltoNewPassManager;
169169
bool mergeArmExidx;
170170
bool mipsN32Abi = false;
171+
bool mmapOutputFile;
171172
bool nmagic;
172173
bool noinhibitExec;
173174
bool nostdlib;

lld/ELF/Driver.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -900,6 +900,8 @@ static void readConfigs(opt::InputArgList &args) {
900900
config->mipsGotSize = args::getInteger(args, OPT_mips_got_size, 0xfff0);
901901
config->mergeArmExidx =
902902
args.hasFlag(OPT_merge_exidx_entries, OPT_no_merge_exidx_entries, true);
903+
config->mmapOutputFile =
904+
args.hasFlag(OPT_mmap_output_file, OPT_no_mmap_output_file, true);
903905
config->nmagic = args.hasFlag(OPT_nmagic, OPT_no_nmagic, false);
904906
config->noinhibitExec = args.hasArg(OPT_noinhibit_exec);
905907
config->nostdlib = args.hasArg(OPT_nostdlib);

lld/ELF/Options.td

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,10 @@ defm merge_exidx_entries: B<"merge-exidx-entries",
232232
"Enable merging .ARM.exidx entries (default)",
233233
"Disable merging .ARM.exidx entries">;
234234

235+
defm mmap_output_file: B<"mmap-output-file",
236+
"Mmap the output file for writing (default)",
237+
"Do not mmap the output file for writing">;
238+
235239
def nmagic: F<"nmagic">, MetaVarName<"<magic>">,
236240
HelpText<"Do not page align sections, link against static libraries.">;
237241

@@ -566,7 +570,6 @@ def: F<"no-add-needed">;
566570
def: F<"no-copy-dt-needed-entries">;
567571
def: F<"no-ctors-in-init-array">;
568572
def: F<"no-keep-memory">;
569-
def: F<"no-mmap-output-file">;
570573
def: F<"no-pipeline-knowledge">;
571574
def: F<"no-warn-mismatch">;
572575
def: Flag<["-"], "p">;

lld/ELF/Writer.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2595,7 +2595,9 @@ template <class ELFT> void Writer<ELFT>::openFile() {
25952595
unlinkAsync(config->outputFile);
25962596
unsigned flags = 0;
25972597
if (!config->relocatable)
2598-
flags = FileOutputBuffer::F_executable;
2598+
flags |= FileOutputBuffer::F_executable;
2599+
if (!config->mmapOutputFile)
2600+
flags |= FileOutputBuffer::F_no_mmap;
25992601
Expected<std::unique_ptr<FileOutputBuffer>> bufferOrErr =
26002602
FileOutputBuffer::create(config->outputFile, fileSize, flags);
26012603

lld/test/ELF/silent-ignore.test

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ RUN: -no-add-needed \
66
RUN: -no-copy-dt-needed-entries \
77
RUN: -no-ctors-in-init-array \
88
RUN: -no-keep-memory \
9-
RUN: -no-mmap-output-file \
109
RUN: -no-pipeline-knowledge \
1110
RUN: -no-warn-mismatch \
1211
RUN: -p \

llvm/include/llvm/Support/FileOutputBuffer.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,10 @@ class FileOutputBuffer {
3232
enum {
3333
/// set the 'x' bit on the resulting file
3434
F_executable = 1,
35+
36+
/// Don't use mmap and instead write an in-memory buffer to a file when this
37+
/// buffer is closed.
38+
F_no_mmap = 2,
3539
};
3640

3741
/// Factory method to create an OutputBuffer object which manages a read/write

llvm/lib/Support/FileOutputBuffer.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,10 @@ FileOutputBuffer::create(StringRef Path, size_t Size, unsigned Flags) {
189189
case fs::file_type::regular_file:
190190
case fs::file_type::file_not_found:
191191
case fs::file_type::status_error:
192-
return createOnDiskBuffer(Path, Size, Mode);
192+
if (Flags & F_no_mmap)
193+
return createInMemoryBuffer(Path, Size, Mode);
194+
else
195+
return createOnDiskBuffer(Path, Size, Mode);
193196
default:
194197
return createInMemoryBuffer(Path, Size, Mode);
195198
}

llvm/unittests/Support/FileOutputBufferTest.cpp

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,28 @@ TEST(FileOutputBuffer, Test) {
118118
EXPECT_TRUE(IsExecutable);
119119
ASSERT_NO_ERROR(fs::remove(File4.str()));
120120

121+
// TEST 5: In-memory buffer works as expected.
122+
SmallString<128> File5(TestDirectory);
123+
File5.append("/file5");
124+
{
125+
Expected<std::unique_ptr<FileOutputBuffer>> BufferOrErr =
126+
FileOutputBuffer::create(File5, 8000, FileOutputBuffer::F_no_mmap);
127+
ASSERT_NO_ERROR(errorToErrorCode(BufferOrErr.takeError()));
128+
std::unique_ptr<FileOutputBuffer> &Buffer = *BufferOrErr;
129+
// Start buffer with special header.
130+
memcpy(Buffer->getBufferStart(), "AABBCCDDEEFFGGHHIIJJ", 20);
131+
ASSERT_NO_ERROR(errorToErrorCode(Buffer->commit()));
132+
// Write to end of buffer to verify it is writable.
133+
memcpy(Buffer->getBufferEnd() - 20, "AABBCCDDEEFFGGHHIIJJ", 20);
134+
// Commit buffer.
135+
ASSERT_NO_ERROR(errorToErrorCode(Buffer->commit()));
136+
}
137+
138+
// Verify file is correct size.
139+
uint64_t File5Size;
140+
ASSERT_NO_ERROR(fs::file_size(Twine(File5), File5Size));
141+
ASSERT_EQ(File5Size, 8000ULL);
142+
ASSERT_NO_ERROR(fs::remove(File5.str()));
121143
// Clean up.
122144
ASSERT_NO_ERROR(fs::remove(TestDirectory.str()));
123145
}

0 commit comments

Comments
 (0)