8298647: GenShen require heap size 2MB granularity #202
Conversation
Generational Shenandoah requires 2MB granularity in order for card
tables to cover the allocated heap. Each byte in a page of card table
represents 512 heap bytes. As card tables are allocated 4KB at a time,
4KB * 512 = 2MB.
There is a circular dependency between the region calculations and the
heap size calculations. This unconditionally rounds up the heap size to
2MB. It might be preferable to do this only when generational mode is
enabled.
Running with:
java -Xlog:gc*=trace -XX:+UseShenandoahGC -mx495m \
-XX:ShenandoahGCMode=generational -version
on a debug build is sufficient to reproduce this problem.
|
👋 Welcome back smonteith! A progress list of the required criteria for merging this PR into |
Webrevs
|
| @@ -724,6 +725,9 @@ size_t ShenandoahHeapRegion::setup_sizes(size_t max_heap_size) { | |||
| FLAG_SET_DEFAULT(ShenandoahMinRegionSize, MIN_REGION_SIZE); | |||
| } | |||
|
|
|||
| // Generational Shenandoah needs this alignment for card tables. | |||
There was a problem hiding this comment.
Thank you for this fix! It would be nice if this constraint were only applied for generation mode, but these sizes are computed quite earlier during startup. You'd need to factor the code out of ShenandoahHeap::initialize_heuristics to know whether the constraint is required at this point.
There was a problem hiding this comment.
Yes, I thought I'd start with a simple fix to highlight the problem first. I experimented with exactly what you suggested, parsing ShenandoahGCMode. I can update this the 2MB alignment isn't desired unconditionally.
There was a problem hiding this comment.
Yes please. We've tried to limit the impact of the generational mode on Shenandoah's other modes.
Only round up for the card table alignment when generational mode is enabled.
|
@stooart-mon This change now passes all automated pre-integration checks. ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details. After integration, the commit message for the final commit will be: You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed. At the time when this comment was updated there had been no new commits pushed to the As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@earthling-amzn) but any other Committer may sponsor as well. ➡️ To flag this PR as ready for integration with the above commit message, type |
|
/integrate |
|
@stooart-mon |
|
/sponsor |
|
Going to push as commit 800c0c8. |
openjdk
bot
removed
ready
|
@earthling-amzn @stooart-mon Pushed as commit 800c0c8. 💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored. |
Generational Shenandoah requires 2MB granularity in order for card tables to cover the allocated heap. Each byte in a page of card table represents 512 heap bytes. As card tables are allocated 4KB at a time, 4KB * 512 = 2MB.
There is a circular dependency between the region calculations and the heap size calculations. This unconditionally rounds up the heap size to 2MB. It might be preferable to do this only when generational mode is enabled.
Running with:
java -Xlog:gc*=trace -XX:+UseShenandoahGC -mx495m
-XX:ShenandoahGCMode=generational -version
on a debug build is sufficient to reproduce this problem.
Progress
Issue
Reviewers
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/shenandoah pull/202/head:pull/202$ git checkout pull/202Update a local copy of the PR:
$ git checkout pull/202$ git pull https://git.openjdk.org/shenandoah pull/202/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 202View PR using the GUI difftool:
$ git pr show -t 202Using diff file
Download this PR as a diff file:
https://git.openjdk.org/shenandoah/pull/202.diff