This is an archive of the discontinued LLVM Phabricator instance.

[Memory] Add basic support for large/huge memory pages
ClosedPublic

Authored by aganea on Feb 27 2019, 8:00 AM.

Details

Summary

This adds Memory::MF_HUGE_HINT which indicates that allocateMappedMemory() shall return a pointer to a large memory page. However the flag is merly a hint because we're not guaranteed in any way that we will get back a large memory page. There are several restrictions:

  • Large/huge memory pages aren't enabled by default on modern OSes (Windows 10 and Linux at least), and should be manually enabled/reserved.
  • Once enabled, it should be kept in mind that large pages are physical only, they can't be swapped.
  • Memory fragmentation can affect the availability of large pages, especially after running the OS for a long time and/or running along many other applications.

allocateMappedMemory() will fallback to 4KB pages if it can't allocate 2MB large pages (when Memory::MF_HUGE_HINT is provided)

Currently, this patch implements Memory::MF_HUGE_HINT only on Windows. The hint will be ignored on Linux, and 4KB pages will always be returned. Unfortunately I don't have a Linux box at hand, and WSL does not seem to support huge pages at the moment.

Also, testing on the build system is a bit tricky. I've added a unit test to exercise the codepath, although I can't ensure the OS will return a large/huge memory page. The test would be too fragile otherwise and could fail occasionally.

Diff Detail

Repository
rL LLVM

Event Timeline

aganea created this revision.Feb 27 2019, 8:00 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 27 2019, 8:00 AM
Herald added a subscriber: kristina. · View Herald Transcript
aganea edited the summary of this revision. (Show Details)Feb 27 2019, 8:01 AM
aganea updated this revision to Diff 188547.Feb 27 2019, 9:01 AM

Fix tests on WSL (Ubuntu).

rnk accepted this revision.Feb 27 2019, 11:23 AM

lgtm

lib/Support/Unix/Memory.inc
48 ↗(On Diff #188547)

At the call site, we ought to add // FIXME: Handle huge page requests..

This revision is now accepted and ready to land.Feb 27 2019, 11:23 AM
zturner added inline comments.Feb 27 2019, 11:38 AM
lib/Support/Windows/Memory.inc
59

Can we mark this function static?

65

Can you do an early out here if it failed?

72

Why do we actually even need to set this privilege on the process token? Will huge pages not work without it?

Also, it's strange to be doing this from a function called getLargePageSize which sounds like it doesn't modify anything.

75

There's a handle leak here.

zturner added inline comments.Feb 27 2019, 11:44 AM
lib/Support/Windows/Memory.inc
72

Actually I found this that explains it:

https://blogs.msdn.microsoft.com/oldnewthing/20110128-00/?p=11643

Is looking up the privilege value and opening the token expensive? If so, maybe you want to put this behind an llvm::call_once()

aganea marked 2 inline comments as done.Feb 27 2019, 12:20 PM
aganea added inline comments.
lib/Support/Windows/Memory.inc
72

getLargePageSize() returns into a static variable (see L114) which shall be thread-safe as per C++11 specification section 6.7.4. I know there were issues previously with VS2012, but any modern C++11/14 compiler should support that (I know for sure it works since VS2015).

Is there any reason for calling llvm::call_once() over static initialization in this case?

aganea updated this revision to Diff 188602.Feb 27 2019, 12:35 PM
aganea marked 5 inline comments as done.

Adressed all comments, except for llvm::call_once.

zturner added inline comments.Feb 27 2019, 1:00 PM
lib/Support/Windows/Memory.inc
102–112

FWIW, this comment about static locals is no longer accurate. I wonder if we could write this as:

static size_t DefaultGranularity = getAllocationGranularity();
static Optional<size_t> LargePageGranularity = getLargePargeGranularity();

DWORD Flags = MEM_RESERVE | MEM_COMMIT;

size_t Granularity = DefaultGranularity;
if ((Flags & MF_HUGE_HINT) && LargePageGranularity.hasValue()) {
  Flags |= MEM_LARGE_PAGES;
  Granularity = *LargePageGranularity;
}
aganea updated this revision to Diff 188616.Feb 27 2019, 1:53 PM
aganea marked 2 inline comments as done.
aganea added inline comments.
lib/Support/Windows/Memory.inc
102–112

I was tempted to change that code at first, but I said maybe in a later commit. Looks much better now - thank you!

aganea updated this revision to Diff 188618.Feb 27 2019, 2:00 PM

Added comments.

zturner accepted this revision.Feb 27 2019, 2:01 PM
This revision was automatically updated to reflect the committed changes.