-
Notifications
You must be signed in to change notification settings - Fork 224
8351500: G1: NUMA migrations cause crashes in region allocation #3607
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
base: master
Are you sure you want to change the base?
Conversation
👋 Welcome back stuefe! A progress list of the required criteria for merging this PR into |
❗ This change is not yet ready to be integrated. |
This backport pull request has now been updated with issue from the original commit. |
/approval request This is a solution for a quite hairy problem that hits customers on NUMA machines. It is rare, intermittent, and quite difficult to pinpoint. I therefore would appreciate it if I got approval for backporting. Risk: lowish. The mechanism is quite clear, the patch, albeit unclean, simple in essence. I also took care to test its functioning well (see JBS description for details.) |
Ping @sjohanss - could I get a review for this? |
Anyone? |
Could I please have reviews? @jerboaa maybe or @gnu-andrew ? |
Yes we can test it. |
This is not a clean backport. The effected G1Allocator and G1Collector methods have changed since JDK17.
So this backports reimplements the patch in a minimally invasive way while retaining as much similarity as possible with the original patch.
The gist of the patch is clear: instead of finding out the NUMA node index at every instance of G1Allocator::allocate_xxx, and then be subject to NUMA node migrations, we fix the NUMA node index once and use that one.
I tested this patch with my "FakeNUMA" addition (I plan to upstream that one at some point). This FakeNUMA mode mimics a lot of NUMA node migrations. I can verify that without this patch the JVM crashes quickly, with the patch it does not crash.
Progress
Issue
Reviewing
Using
git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk17u-dev.git pull/3607/head:pull/3607
$ git checkout pull/3607
Update a local copy of the PR:
$ git checkout pull/3607
$ git pull https://git.openjdk.org/jdk17u-dev.git pull/3607/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 3607
View PR using the GUI difftool:
$ git pr show -t 3607
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk17u-dev/pull/3607.diff
Using Webrev
Link to Webrev Comment