Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Large pages support in GC #23251

Merged
merged 1 commit into from Apr 8, 2019

Conversation

Projects
None yet
5 participants
@mjsabby
Copy link
Member

commented Mar 14, 2019

Fixes #18371.

@mjsabby

This comment has been minimized.

Copy link
Member Author

commented Mar 14, 2019

I'm thinking if a better error message should be presented if SeLockMemoryPrivilege is missing as opposed to returning OutOfMemory. My personal opinion is that documentation should be good enough to use this advanced feature rather than plumbing a specific error message out of the runtime.

But maybe we could tackle it in the host or CLI?

@mjsabby

This comment has been minimized.

Copy link
Member Author

commented Mar 14, 2019

@Maoni0 I was wrong, Linux does have a concept of Reserve and Commit. PROT_NONE flag in the mmap call is like VirtualReserve. So I have to fix my code accordingly.

Show resolved Hide resolved src/gc/env/gcenv.os.h Outdated
Show resolved Hide resolved src/gc/gc.cpp Outdated
Show resolved Hide resolved src/gc/unix/gcenv.unix.cpp Outdated
Show resolved Hide resolved src/gc/gc.cpp Outdated
@Maoni0

This comment has been minimized.

Copy link
Member

commented Mar 14, 2019

@mjsabby one thing I missed was we should simply not do anything for gc_heap::decommit_heap_segment_pages 'cause our premise is you grab large pages upfront and don't let them go. I suspect you will get an error if you try to decommit on windows anyway. you could do

void gc_heap::decommit_heap_segment_pages (heap_segment* seg,
                                           size_t extra_space)
{
    if (use_large_pages_p)
        return;
Show resolved Hide resolved src/vm/gcenv.os.cpp Outdated

@mjsabby mjsabby force-pushed the mjsabby:largePagesInGC branch from 07d48af to 68b3796 Mar 25, 2019

@mjsabby

This comment has been minimized.

Copy link
Member Author

commented Mar 25, 2019

I have to introduce OpenProcessToken, AdjutTokenPrivileges and LookupPrivilegeValue for the Windows build. I'm going to do LoadLibrary advapi32 and then resolve those functions. I could also put them in utilcode but I don't think there is any utilcode dependency in the GC->EE/OS interface today.

@janvorli

This comment has been minimized.

Copy link
Member

commented Mar 25, 2019

I'm going to do LoadLibrary advapi32 and then resolve those functions.

I don't see a reason for using LoadLibrary for these, you can use them directly. They are supported since Windows XP.

@janvorli

This comment has been minimized.

Copy link
Member

commented Mar 25, 2019

OK, I can see you have already done it that way.

@mjsabby mjsabby force-pushed the mjsabby:largePagesInGC branch 6 times, most recently from bbfed48 to 86b01c7 Mar 27, 2019

@mjsabby

This comment has been minimized.

Copy link
Member Author

commented Mar 27, 2019

@Maoni0 What are your thoughts on hard limit + numa awareness + now large pages? I'm thinking to start with we keep Large Pages NUMA-unaware, otherwise it gets a bit complicated where and how much should the memory be split and commuted at startup. Already the multiple heaps overcommits by 3X I think.

For 3.0 maybe we start with GCHardLimit + Large Pages does a 3X overcommit at startup then in the future we add configs for splitting the memory across LOH, SOH + NUMA Nodes?

@Maoni0

This comment has been minimized.

Copy link
Member

commented Mar 28, 2019

For 3.0 maybe we start with GCHardLimit + Large Pages does a 3X overcommit at startup

this sounds fine to me if it's fine with you since you are our 1st customer for this :) also I'm working on reducing this 3x to 2x.

then in the future we add configs for splitting the memory across LOH, SOH + NUMA Nodes?

most likely we wouldn't need another config - it'll just be improved naturally in the future versions.

@mjsabby mjsabby force-pushed the mjsabby:largePagesInGC branch 4 times, most recently from 603cce1 to 77efbd4 Mar 29, 2019

@mjsabby mjsabby changed the title [WIP] Add Large pages support in GC Add Large pages support in GC Mar 29, 2019

@mjsabby

This comment has been minimized.

Copy link
Member Author

commented Mar 29, 2019

This is ready for review again.

After this change, the situation will be:

Memory Type Windows Linux
GC Heap ✔️
Card Table
Handle Table
Jitted Code
Other runtime heaps

Next up will be Large Pages support for the GC Heap on Linux.

Documentation Items:

(1) Noting that GCHeapHardLimit is required (GCHeapHardLimit takes input as hex bytes)
(2) SeLockMemoryPrivelege notes for Windows.
(3) For Linux we may have to talk about /proc/sys/vm/overcommit_memory which can be there on some distros, I'm not sure on the implications yet.
(4) MiniDump will be the size of the preallocated gc heap + some ... so they will be quite a bit larger.

Show resolved Hide resolved src/gc/gc.cpp Outdated
@Maoni0

This comment has been minimized.

Copy link
Member

commented Apr 2, 2019

LGTM - comments above.

Show resolved Hide resolved src/gc/unix/gcenv.unix.cpp Outdated
Show resolved Hide resolved src/gc/unix/gcenv.unix.cpp
Show resolved Hide resolved src/gc/windows/gcenv.windows.cpp Outdated
Show resolved Hide resolved src/gc/windows/gcenv.windows.cpp Outdated
Show resolved Hide resolved src/vm/gcenv.os.cpp Outdated
Show resolved Hide resolved src/vm/gcenv.os.cpp Outdated

@mjsabby mjsabby force-pushed the mjsabby:largePagesInGC branch 2 times, most recently from 574b9af to 28ae9ab Apr 3, 2019

@mjsabby

This comment has been minimized.

Copy link
Member Author

commented Apr 4, 2019

@Maoni0 @janvorli I've made the requested updates, although I'm still trying to see why my feature detection for restricting to Linux TLB isn't working as expected and failing the macOS build.

Show resolved Hide resolved src/gc/unix/gcenv.unix.cpp Outdated

@mjsabby mjsabby force-pushed the mjsabby:largePagesInGC branch 2 times, most recently from ad544ed to ae4f3f1 Apr 4, 2019

@mjsabby

This comment has been minimized.

Copy link
Member Author

commented Apr 5, 2019

/azp run coreclr-ci

@mjsabby

This comment has been minimized.

Copy link
Member Author

commented Apr 5, 2019

How does one trigger these CI jobs again?

@mjsabby

This comment has been minimized.

Copy link
Member Author

commented Apr 5, 2019

@dotnet-bot test coreclr-ci

@mjsabby

This comment has been minimized.

Copy link
Member Author

commented Apr 5, 2019

@dotnet-bot test Ubuntu x64 Checked CoreFX Tests

@mjsabby

This comment has been minimized.

Copy link
Member Author

commented Apr 5, 2019

@dotnet-bot test coreclr-ci (Test Pri0 Windows_NT x64 checked)

@mjsabby mjsabby force-pushed the mjsabby:largePagesInGC branch from ae4f3f1 to c4d5f5d Apr 5, 2019

@mjsabby

This comment has been minimized.

Copy link
Member Author

commented Apr 5, 2019

@Maoni0 @janvorli Updated with your feedback.

@Maoni0

This comment has been minimized.

Copy link
Member

commented Apr 5, 2019

LGTM.

@mjsabby

This comment has been minimized.

Copy link
Member Author

commented Apr 5, 2019

The coreclr-ci says the following:

Failed: 0 (0.00 %)
Passed: 35,542 (100.00 %)
Other: 0 (0.00 %)
Total: 35,542

But it still marked as failed.

@mjsabby

This comment has been minimized.

Copy link
Member Author

commented Apr 5, 2019

Looks like the error is

##[Error 1]
The agent: Hosted macOS 8 lost communication with the server. Verify the machine is running and has a healthy network connection. For more information, see: https://go.microsoft.com/fwlink/?linkid=846610

@mjsabby

This comment has been minimized.

Copy link
Member Author

commented Apr 5, 2019

@dotnet-bot test coreclr-ci (Test Pri0 OSX x64 checked)

@mjsabby mjsabby force-pushed the mjsabby:largePagesInGC branch 2 times, most recently from 2612370 to d36896f Apr 6, 2019

@mjsabby mjsabby force-pushed the mjsabby:largePagesInGC branch from d36896f to d33f73f Apr 8, 2019

@janvorli janvorli merged commit 1874101 into dotnet:master Apr 8, 2019

41 checks passed

Ubuntu arm Cross Checked crossgen_comparison Build and Test Build finished.
Details
Ubuntu arm Cross Release crossgen_comparison Build and Test Build finished.
Details
Ubuntu x64 Checked CoreFX Tests Build finished.
Details
Ubuntu x64 Formatting Build finished.
Details
WIP Ready for review
Details
Windows_NT x64 Checked CoreFX Tests Build finished.
Details
Windows_NT x64 Formatting Build finished.
Details
Windows_NT x64 Release CoreFX Tests Build finished.
Details
Windows_NT x64 full_opt ryujit CoreCLR Perf Tests Correctness Build finished.
Details
Windows_NT x64 min_opt ryujit CoreCLR Perf Tests Correctness Build finished.
Details
Windows_NT x86 full_opt ryujit CoreCLR Perf Tests Correctness Build finished.
Details
Windows_NT x86 min_opt ryujit CoreCLR Perf Tests Correctness Build finished.
Details
coreclr-ci Build #20190408.717 succeeded
Details
coreclr-ci (Build Linux arm checked) Build Linux arm checked succeeded
Details
coreclr-ci (Build Linux arm64 checked) Build Linux arm64 checked succeeded
Details
coreclr-ci (Build Linux arm64 release) Build Linux arm64 release succeeded
Details
coreclr-ci (Build Linux_musl x64 checked) Build Linux_musl x64 checked succeeded
Details
coreclr-ci (Build Linux_musl x64 release) Build Linux_musl x64 release succeeded
Details
coreclr-ci (Build Linux_rhel6 x64 release) Build Linux_rhel6 x64 release succeeded
Details
coreclr-ci (Build Windows_NT arm Checked) Build Windows_NT arm Checked succeeded
Details
coreclr-ci (Build Windows_NT arm64 Checked) Build Windows_NT arm64 Checked succeeded
Details
coreclr-ci (Build Windows_NT x64 Checked) Build Windows_NT x64 Checked succeeded
Details
coreclr-ci (Build Windows_NT x64 Debug) Build Windows_NT x64 Debug succeeded
Details
coreclr-ci (Build Windows_NT x86 Checked) Build Windows_NT x86 Checked succeeded
Details
coreclr-ci (Build Windows_NT x86 Debug) Build Windows_NT x86 Debug succeeded
Details
coreclr-ci (Test Pri0 Linux x64 checked) Test Pri0 Linux x64 checked succeeded
Details
coreclr-ci (Test Pri0 Linux_musl x64 release) Test Pri0 Linux_musl x64 release succeeded
Details
coreclr-ci (Test Pri0 OSX x64 checked) Test Pri0 OSX x64 checked succeeded
Details
coreclr-ci (Test Pri0 Windows_NT arm checked) Test Pri0 Windows_NT arm checked succeeded
Details
coreclr-ci (Test Pri0 Windows_NT arm64 checked) Test Pri0 Windows_NT arm64 checked succeeded
Details
coreclr-ci (Test Pri0 Windows_NT x64 checked) Test Pri0 Windows_NT x64 checked succeeded
Details
coreclr-ci (Test Pri0 Windows_NT x86 checked) Test Pri0 Windows_NT x86 checked succeeded
Details
coreclr-ci (Test pri0 Linux arm checked) Test pri0 Linux arm checked succeeded
Details
coreclr-ci (Test pri0 Linux arm64 checked) Test pri0 Linux arm64 checked succeeded
Details
coreclr-ci (Test pri0 Linux_musl x64 checked) Test pri0 Linux_musl x64 checked succeeded
Details
coreclr-ci (build Linux x64 Checked) build Linux x64 Checked succeeded
Details
coreclr-ci (build OSX x64 Checked) build OSX x64 Checked succeeded
Details
coreclr-ci (build Windows_NT arm Release) build Windows_NT arm Release succeeded
Details
coreclr-ci (build Windows_NT arm64 Release) build Windows_NT arm64 Release succeeded
Details
coreclr-ci (build Windows_NT x64 Release) build Windows_NT x64 Release succeeded
Details
license/cla All CLA requirements met.
Details

@mjsabby mjsabby deleted the mjsabby:largePagesInGC branch Apr 8, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.