Skip to content
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 23 additions & 10 deletions lld/MachO/Driver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
#include "llvm/Object/Archive.h"
#include "llvm/Option/ArgList.h"
#include "llvm/Support/CommandLine.h"
#include "llvm/Support/Debug.h"
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/Parallel.h"
#include "llvm/Support/Path.h"
Expand All @@ -53,6 +54,10 @@
#include "llvm/TextAPI/Architecture.h"
#include "llvm/TextAPI/PackedVersion.h"

#if !_WIN32
#include <sys/mman.h>
#endif

using namespace llvm;
using namespace llvm::MachO;
using namespace llvm::object;
Expand Down Expand Up @@ -334,29 +339,35 @@ class SerialBackgroundQueue {
// This code forces the page-ins on multiple threads so
// the process is not stalled waiting on disk buffer i/o.
void multiThreadedPageInBackground(DeferredFiles &deferred) {
using namespace std::chrono;
static const size_t pageSize = Process::getPageSizeEstimate();
static const size_t largeArchive = 10 * 1024 * 1024;
#ifndef NDEBUG
using namespace std::chrono;
std::atomic_int numDeferedFilesTouched = 0;
static std::atomic_uint64_t totalBytes = 0;
std::atomic_int numDeferedFilesAdvised = 0;
auto t0 = high_resolution_clock::now();
#endif

auto preloadDeferredFile = [&](const DeferredFile &deferredFile) {
const StringRef &buff = deferredFile.buffer.getBuffer();
if (buff.size() > largeArchive)
return;
#ifndef NDEBUG

totalBytes += buff.size();
numDeferedFilesTouched += 1;
#endif
numDeferedFilesAdvised += 1;

#if _WIN32
// Reference all file's mmap'd pages to load them into memory.
for (const char *page = buff.data(), *end = page + buff.size(); page < end;
page += pageSize)
LLVM_ATTRIBUTE_UNUSED volatile char t = *page;
#else
#define DEBUG_TYPE "lld-madvise"
auto aligned = llvm::alignAddr(buff.data(), Align(pageSize));
if (madvise((void *)aligned, buff.size(), MADV_WILLNEED) < 0)
LLVM_DEBUG(llvm::dbgs() << "madvise error: " << strerror(errno) << "\n");
#undef DEBUG_TYPE
#endif
};

#if LLVM_ENABLE_THREADS
{ // Create scope for waiting for the taskGroup
std::atomic_size_t index = 0;
Expand All @@ -371,14 +382,16 @@ void multiThreadedPageInBackground(DeferredFiles &deferred) {
}
});
}
#else
for (const auto &file : deferred)
preloadDeferredFile(file);
#endif
#ifndef NDEBUG

auto dt = high_resolution_clock::now() - t0;
if (Process::GetEnv("LLD_MULTI_THREAD_PAGE"))
llvm::dbgs() << "multiThreadedPageIn " << totalBytes << "/"
<< numDeferedFilesTouched << "/" << deferred.size() << "/"
<< numDeferedFilesAdvised << "/" << deferred.size() << "/"
<< duration_cast<milliseconds>(dt).count() / 1000. << "\n";
#endif
}

static void multiThreadedPageIn(const DeferredFiles &deferred) {
Expand Down
6 changes: 4 additions & 2 deletions llvm/lib/Support/MemoryBuffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -505,8 +505,10 @@ getOpenFileImpl(sys::fs::file_t FD, const Twine &Filename, uint64_t FileSize,
// On at least Linux, and possibly on other systems, mmap may return pages
// from the page cache that are not properly filled with trailing zeroes,
// if some prior user of the page wrote non-zero bytes. Detect this and
// don't use mmap in that case.
if (!RequiresNullTerminator || *Result->getBufferEnd() == '\0')
// don't use mmap in that case (unless it is object or archive file).
if (!RequiresNullTerminator || *Result->getBufferEnd() == '\0' ||
StringRef(Filename.str()).ends_with(".o") ||
StringRef(Filename.str()).ends_with(".a"))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure how I feel about leaking object file logic into MemoryBuffer.cpp. Could we just use RequiresNullTerminator = false in getFile() for object files (or however this code is called)?

Copy link
Contributor Author

@johnno1962 johnno1962 Sep 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know, this version is awful but at least the fix is located close to the change that caused the problem. Have a look at the commit to see the previous version which passed in RequiresNullTerminator = false.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any ideas how we should handle this @zygoloid?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why would we be setting RequiresNullTerminator when mapping a .o or .a file? That sounds like a bug in the caller to me.

return std::move(Result);
}
}
Expand Down