8353184: ZGC: Simplify and correct tlab_used() tracking #24814

kstefanj · 2025-04-23T07:58:35Z

Please review this change to improve TLAB handling in ZGC.

Summary
In ZGC the maximum TLAB size is 256k and in many cases we want the TLABs to be this big. But for threads only allocating a fraction of this, using TLABs of this size will render significant waste. This is normally handled by the shared TLAB sizing heuristic, but there have been a few things in ZGC which have prevented this mechanism to work as expected.

The heuristic bases the resizing on several things, and the GC is responsible for providing the amount used memory for TLABs (tlab_used()) and the capacity available for TLABs (tlab_capacity()). Capacity is more or less the size of Eden for the other GCs, but ZGC does not have any generation sizes there is no given size for Eden. Before this change we returned the heap capacity as the TLAB capacity, since in theory we can use what is left for TLABs. Returning this, more or less disables the sizing heuristic since we only sample the usage when this holds:

bool update_allocation_history = used > 0.5 * capacity;

So we need to come up with a better value to return as capacity, we could use the amount of free memory, but this is also an over estimation of what will actually be used. The proposed approach is to use an average over the last 10 values of what was actually used for TLABs as the capacity. This will provide a good estimate of what the expected TLAB capacity is and the sizing heuristic will work as expected.

Another problem in this area is that since ZGC does TLAB retiring concurrently, the used value returned has previously been reset before used in the sizing heuristic. So to be able to use consisten values, we need to snapshot the usage in the mark start pause for the young generation and use those value for any TLAB retired after this pause.

How we track the TLAB used value is also changed. Before this change, TLAB used was tracked per-cpu and the way it was implemented let to some unwanted overhead. We added two additional fields that were tracked for all ages, but only used for Eden. These fields were cleared in the mark start pause, and when having many CPUs this actually affect the pause time. The new code tracks the Eden usage in the page-allocator instead.

This change also fixes to that the maximum TLAB size returned from ZGC is in words not bytes, which will mostly help logging, since the actual sizing is still enforced correctly.

Testing

Functional testing tier1-tier7
Performance testing in Aurora is neutral
Manual testing looking at TLAB waste shows a clear reduction, in some scenarios the waste could previously be above 2% and now it is below 1%
Manual verification that the worse case pauses are shorter due to the reduced work in the mark start pause

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Commit message must refer to an issue

Issue

JDK-8353184: ZGC: Simplify and correct tlab_used() tracking (Enhancement - P3)

Reviewers

Stefan Karlsson (@stefank - Reviewer)
Axel Boldt-Christmas (@xmas92 - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/24814/head:pull/24814
$ git checkout pull/24814

Update a local copy of the PR:
$ git checkout pull/24814
$ git pull https://git.openjdk.org/jdk.git pull/24814/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 24814

View PR using the GUI difftool:
$ git pr show -t 24814

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/24814.diff

Using Webrev

Link to Webrev Comment

bridgekeeper · 2025-04-23T07:59:31Z

👋 Welcome back sjohanss! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2025-04-23T07:59:44Z

@kstefanj 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:

8353184: ZGC: Simplify and correct tlab_used() tracking

Reviewed-by: stefank, aboldtch

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 326 new commits pushed to the master branch:

a091e2f: 8351029: IncludeCustomExtension does not work on cygwin with source code below /home
3aa2ea7: 8301971: Make JDK source code UTF-8
74e981e: 8353735: [JVMCI] Allow specifying storage kind of the callee save register
... and 323 more: https://git.openjdk.org/jdk/compare/50358d1ca49c26d100c5c658de29c75f864fdc60...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

openjdk · 2025-04-23T08:00:24Z

@kstefanj The following label will be automatically applied to this pull request:

hotspot-gc

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

mlbridge · 2025-04-23T08:03:40Z

Webrevs

stefank

I like this change. I've added a few comments below.

stefank · 2025-05-08T15:56:23Z

src/hotspot/share/gc/z/zTLABUsage.cpp

+
+

Suggested change

stefank · 2025-05-08T15:56:41Z

src/hotspot/share/gc/z/zTLABUsage.cpp

+  precond(size <= _used);
+  Atomic::sub(&_used, size, memory_order_relaxed);


Suggested change

precond(size <= _used);

Atomic::sub(&_used, size, memory_order_relaxed);

precond(size <= _used);

Atomic::sub(&_used, size, memory_order_relaxed);

stefank · 2025-05-08T15:57:19Z

src/hotspot/share/gc/z/zTLABUsage.cpp

+}
+
+void ZTLABUsage::reset() {
+  const size_t current_used = Atomic::xchg(&_used, (size_t) 0);


Does this work instead?

Suggested change

const size_t current_used = Atomic::xchg(&_used, (size_t) 0);

const size_t current_used = Atomic::xchg(&_used, 0u);

No, 0ul works on Linux, but Windows fails with that.

stefank · 2025-05-08T16:01:08Z

src/hotspot/share/gc/z/zTLABUsage.cpp

+  }
+
+  // Save the old values for logging
+  const size_t old_used = used();


It's not immediately obvious what _used is compared to used() Could one of these be renamed so that readers don't mistakenly assume that used() returns _used.

Talked a bit about this offline, will add some comments and rename used() and capacity() to tlab_used() and tlab_capacity() to make it a bit more clear that they are not directly connected and also better match the ZHeap interface.

xmas92

lgtm.

kstefanj · 2025-05-13T07:43:50Z

Thanks for the reviews @stefank and @xmas92

/integrate

openjdk · 2025-05-13T07:44:35Z

Going to push as commit 526f543.
Since your change was applied there have been 375 commits pushed to the master branch:

03dca03: 8354908: javac mishandles supplementary character in character literal
6e08469: 8356658: java/foreign/TestBufferStackStress2.java failed again with junit action timed out
24d77ad: 8356057: PrintingProcessor (-Xprint) does not print type variable bounds and type annotations for Object supertypes
... and 372 more: https://git.openjdk.org/jdk/compare/50358d1ca49c26d100c5c658de29c75f864fdc60...master

Your commit was automatically rebased without conflicts.

openjdk · 2025-05-13T07:44:41Z

@kstefanj Pushed as commit 526f543.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

kstefanj added 8 commits April 22, 2025 10:05

Max tlab size should be reported in words

57c2542

Add class to keep track of TLAB usage

554a0de

Track eden-usage in page allocator

9276951

Fixes for TLABUsage

7421569

Fixes for eden tracking

1b132bf

Junba-space

4e7ea16

More TLABUsage fixes

c96c5a8

Change memory order to relaxed

0c1f6ee

openjdk bot added the rfr Pull request is ready for review label Apr 23, 2025

openjdk bot added the hotspot-gc hotspot-gc-dev@openjdk.org label Apr 23, 2025

openjdk bot mentioned this pull request May 3, 2025

collaborater #25022

Closed

3 tasks

kstefanj added 5 commits May 6, 2025 10:44

Default TLAB size of 8k, avoid 0 updates and reasonable starting values

76c79f5

Revert initial value for TLABUsage

4889af7

Keep all TLAB tracking in TLABUsage

10d455f

Problemlist heap sampling test

f361fc5

Handle inc and dec in alloc/undo

ba7cb67

stefank reviewed May 8, 2025

View reviewed changes

StefanK review

2f5742f

stefank approved these changes May 9, 2025

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label May 9, 2025

xmas92 approved these changes May 9, 2025

View reviewed changes

openjdk bot added the integrated Pull request has been integrated label May 13, 2025

openjdk bot closed this May 13, 2025

openjdk bot removed the ready Pull request is ready to be integrated label May 13, 2025

openjdk bot removed the rfr Pull request is ready for review label May 13, 2025

		precond(size <= _used);
		Atomic::sub(&_used, size, memory_order_relaxed);

	const size_t current_used = Atomic::xchg(&_used, (size_t) 0);
	const size_t current_used = Atomic::xchg(&_used, 0u);

8353184: ZGC: Simplify and correct tlab_used() tracking #24814

8353184: ZGC: Simplify and correct tlab_used() tracking #24814

Uh oh!

Conversation

kstefanj commented Apr 23, 2025 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewers

Reviewing

Uh oh!

bridgekeeper bot commented Apr 23, 2025

Uh oh!

openjdk bot commented Apr 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openjdk bot commented Apr 23, 2025

Uh oh!

mlbridge bot commented Apr 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

stefank left a comment

Choose a reason for hiding this comment

Uh oh!

stefank May 8, 2025

Choose a reason for hiding this comment

Uh oh!

stefank May 8, 2025

Choose a reason for hiding this comment

Uh oh!

stefank May 8, 2025

Choose a reason for hiding this comment

Uh oh!

kstefanj May 9, 2025

Choose a reason for hiding this comment

Uh oh!

stefank May 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kstefanj May 9, 2025

Choose a reason for hiding this comment

Uh oh!

xmas92 left a comment

Choose a reason for hiding this comment

Uh oh!

kstefanj commented May 13, 2025

Uh oh!

openjdk bot commented May 13, 2025

Uh oh!

openjdk bot commented May 13, 2025

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

kstefanj commented Apr 23, 2025 •

edited by openjdk bot

Loading

openjdk bot commented Apr 23, 2025 •

edited

Loading

mlbridge bot commented Apr 23, 2025 •

edited

Loading

stefank May 8, 2025 •

edited

Loading