Skip to content

Conversation

xmas92
Copy link
Member

@xmas92 xmas92 commented Sep 19, 2025

JDK-8365053 made a fact which is already well known to ZGC developers clear. We pull in large parts of the ZGC implementation through the access API, via zbarrierset.inline.hpp.

ZGC developers are well aware as touching most .hpp or inline.hpp files in gc/z requires rebuilding most of hotspot in incremental builds.

I propose we create a boundary between the barrier set and the implementation. The main reason being making incremental builds less painful.

I experimented with this last year, at the time I saw no real difference in full build times, nor any performance regressions from not inlining the barrier implementation into the access API.

Will reevaluate the performance implications.

I ran the bin/update_pch.sh script, but with the default MIN_MS I saw the same list both before and after this change:

#include "gc/g1/g1BarrierSet.inline.hpp"
#include "gc/shenandoah/shenandoahHeap.inline.hpp"
#include "memory/iterator.inline.hpp"
#include "oops/access.hpp"
#include "oops/access.inline.hpp"
#include "oops/oop.inline.hpp"
#include "utilities/globalDefinitions.hpp"

However when running with MIN_MS reduced by an order of magnitude #include "gc/z/zBarrier.inline.hpp" was included without this patch, and was excluded after with this patch.

Also cross-compiled ppc64le, s390x and riscv64 (fast debug). Could not find any missing includes, have not built all configurations.

For some reason windows slow debug failed to build because test/hotspot/gtest/runtime/test_os_windows.cpp was missing os_windows.hpp, did not investigate further, but included runtime/os.inline.hpp in the test as it includes all OS and OS CPU specific declarations and inline definitions.

  • Testing
    • Tier 1 + ZGC tier 1-5 on Oracle supported platforms

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-8367972: ZGC: Reduce ZBarrierSet includes (Enhancement - P4)

Reviewers

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 27386

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

Using diff file

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

Using Webrev

Link to Webrev Comment

@bridgekeeper
Copy link

bridgekeeper bot commented Sep 19, 2025

👋 Welcome back aboldtch! 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
Copy link

openjdk bot commented Sep 19, 2025

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

8367972: ZGC: Reduce ZBarrierSet includes

Reviewed-by: stefank, eosterlund

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

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
Copy link

openjdk bot commented Sep 19, 2025

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

  • hotspot

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.

@openjdk openjdk bot added the rfr Pull request is ready for review label Sep 19, 2025
@mlbridge
Copy link

mlbridge bot commented Sep 19, 2025

Webrevs

Copy link
Member

@stefank stefank left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good if the performance evaluation looks good.

@openjdk openjdk bot added the ready Pull request is ready to be integrated label Sep 19, 2025
Copy link
Contributor

@fisk fisk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

@fandreuz
Copy link
Contributor

fandreuz commented Sep 19, 2025

Hi @xmas92, some more data about this change.

Before

./ClangBuildAnalyzer/build/ClangBuildAnalyzer --analyze cba_out_20250919-1055 | grep /z/ -A 3
126184 ms: /jdk/src/hotspot/share/gc/z/zGeneration.inline.hpp (included 646 times, avg 195 ms), included via:
  81x: oop.inline.hpp iterator.inline.hpp access.inline.hpp barrierSetConfig.inline.hpp zBarrierSet.inline.hpp zBarrier.inline.hpp
  ...

121644 ms: /jdk/src/hotspot/share/gc/z/zBarrier.inline.hpp (included 646 times, avg 188 ms), included via:
  81x: oop.inline.hpp iterator.inline.hpp access.inline.hpp barrierSetConfig.inline.hpp zBarrierSet.inline.hpp
  ...

109125 ms: /jdk/src/hotspot/share/gc/z/zHeap.inline.hpp (included 646 times, avg 168 ms), included via:
  81x: oop.inline.hpp iterator.inline.hpp access.inline.hpp barrierSetConfig.inline.hpp zBarrierSet.inline.hpp zBarrier.inline.hpp zGeneration.inline.hpp
  ...

--
61224 ms: /jdk/src/hotspot/share/gc/z/zForwardingTable.inline.hpp (included 646 times, avg 94 ms), included via:
  81x: oop.inline.hpp iterator.inline.hpp access.inline.hpp barrierSetConfig.inline.hpp zBarrierSet.inline.hpp zBarrier.inline.hpp zGeneration.inline.hpp zHeap.inline.hpp
  ...

--
58889 ms: /jdk/src/hotspot/share/gc/z/zForwarding.inline.hpp (included 646 times, avg 91 ms), included via:
  81x: oop.inline.hpp iterator.inline.hpp access.inline.hpp barrierSetConfig.inline.hpp zBarrierSet.inline.hpp zBarrier.inline.hpp zGeneration.inline.hpp zHeap.inline.hpp zForwardingTable.inline.hpp
  ...

After

./ClangBuildAnalyzer/build/ClangBuildAnalyzer --analyze after | grep /z/ -A 3
31714 ms: /jdk/src/hotspot/share/gc/z/zHeap.inline.hpp (included 40 times, avg 792 ms), included via:
  13x: zBarrier.inline.hpp zGeneration.inline.hpp
  ...

--
31012 ms: /jdk/src/hotspot/share/gc/z/zForwarding.inline.hpp (included 40 times, avg 775 ms), included via:
  13x: zBarrier.inline.hpp zGeneration.inline.hpp zHeap.inline.hpp zForwardingTable.inline.hpp
  ...

--
27089 ms: /jdk/src/hotspot/share/gc/z/zForwardingTable.inline.hpp (included 40 times, avg 677 ms), included via:
  13x: zBarrier.inline.hpp zGeneration.inline.hpp zHeap.inline.hpp
  ...

27058 ms: /jdk/src/hotspot/share/gc/z/zGeneration.inline.hpp (included 40 times, avg 676 ms), included via:
  13x: zBarrier.inline.hpp
  ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hotspot [email protected] ready Pull request is ready to be integrated rfr Pull request is ready for review
Development

Successfully merging this pull request may close these issues.

4 participants