8307346 - Add missing gc+phases logging for ObjectCount(AfterGC) JFR event collection code #13772

olivergillespie · 2023-05-03T10:04:14Z

Add logging for the time taken to collect/report ObjectCount(AfterGC) event to Serial, Parallel and G1 full collectors, for parity with the G1 concurrent collector. This event can be very expensive to collect (single threaded STW full heap scan), so it's very important to show it clearly to the user.

Added output:

Serial

[7.976s][debug][gc,phases      ] GC(7) Trigger cleanups 0.002ms
[7.977s][debug][gc,phases      ] GC(7) Class Unloading 0.556ms
++ [7.977s][debug][gc,phases,start] GC(7) Report Object Count
++ [8.529s][debug][gc,phases      ] GC(7) Report Object Count 552.065ms
[8.529s][info ][gc,phases      ] GC(7) Phase 1: Mark live objects 1066.882ms
[8.529s][info ][gc,phases,start] GC(7) Phase 2: Compute new object addresses

Parallel

[5.786s][debug][gc,phases      ] GC(12) Trigger cleanups 0.002ms
[5.786s][debug][gc,phases      ] GC(12) Class Unloading 0.556ms
++ [5.786s][debug][gc,phases,start] GC(12) Report Object Count
++ [6.313s][debug][gc,phases      ] GC(12) Report Object Count 526.307ms
[6.313s][info ][gc,phases      ] GC(12) Marking Phase 889.900ms
[6.313s][info ][gc,phases,start] GC(12) Summary Phase

G1 Full

[3.922s][debug][gc,phases         ] GC(24) Trigger cleanups 0.002ms
[3.922s][debug][gc,phases         ] GC(24) Phase 1: Class Unloading and Cleanup 0.159ms
++ [3.922s][debug][gc,phases,start   ] GC(24) Report Object Count
++ [4.442s][debug][gc,phases         ] GC(24) Report Object Count 519.292ms
[4.442s][info ][gc,phases         ] GC(24) Phase 1: Mark live objects 653.209ms
[4.442s][info ][gc,phases,start   ] GC(24) Phase 2: Prepare compaction

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-8307346: Add missing gc+phases logging for ObjectCount(AfterGC) JFR event collection code

Reviewers

Thomas Schatzl (@tschatzl - Reviewer) ⚠️ Review applies to 3ef4f4cc
Aleksey Shipilev (@shipilev - Reviewer)
Albert Mingkun Yang (@albertnetymk - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 13772

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

Using diff file

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

Webrev

Link to Webrev Comment

…event collection code

bridgekeeper · 2023-05-03T10:05:28Z

👋 Welcome back olivergillespie! 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 · 2023-05-03T10:07:17Z

@olivergillespie 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 · 2023-05-03T10:11:22Z

Webrevs

01: Full - Incremental (ce7227c5)
00: Full (3ef4f4cc)

tschatzl

Lgtm.

shipilev

Some nits.

src/hotspot/share/gc/parallel/psParallelCompact.cpp

src/hotspot/share/gc/serial/genMarkSweep.cpp

openjdk · 2023-05-03T10:17:12Z

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

8307346: Add missing gc+phases logging for ObjectCount(AfterGC) JFR event collection code

Reviewed-by: tschatzl, shade, ayang

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:

e206d57: 8307362: Remove test com/sun/jdi/JdbLastErrorTest.java
82a8e91: 8306489: Open source AWT List related tests
465bdd9: 8307091: A few client tests intermittently throw ConcurrentModificationException
caee1be: 8307305: Update debugger tests to support JTREG_TEST_THREAD_FACTORY mode
3599448: 8307147: [x86] Dangling pointer warning for Assembler::_attributes
64ac9a0: 8306946: jdk/test/lib/process/ProcessToolsStartProcessTest.java fails with "wrong number of lines in OutputAnalyzer output"
03030d4: 8307134: Add GTS root CAs
63cd0a3: 4200096: OffScreenImageSource.removeConsumer NullPointerException
db8b3cd: 8305963: Typo in java.security.Security.getProperty
dcb2f3f: 8306320: BufferedImage spec needs clarification w.r.t its implementation of the WritableRenderedImage interface
... and 19 more: https://git.openjdk.org/jdk/compare/a8bf2acb7db63b508ef169e42a27b9c99178cbb1...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.

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 (@tschatzl, @shipilev, @albertnetymk) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

albertnetymk · 2023-05-03T10:22:52Z

single threaded STW full heap scan

HeapInspection::populate_table can use multiple threads. Could report_object_count_after_gc invoke the parallel version?

olivergillespie · 2023-05-03T10:27:59Z

Could report_object_count_after_gc invoke the parallel version?

Yes, I was just thinking the same thing! I think it could, I will follow up to implement that change, thanks for the suggestion.

shipilev · 2023-05-03T10:31:12Z

Could report_object_count_after_gc invoke the parallel version?

Yes, I was just thinking the same thing! I think it could, I will follow up to implement that change, thanks for the suggestion.

Filed: https://bugs.openjdk.org/browse/JDK-8307348

shipilev

This looks fine to me.

olivergillespie · 2023-05-04T09:14:34Z

/integrate

openjdk · 2023-05-04T09:15:37Z

@olivergillespie
Your change (at version ce7227c) is now ready to be sponsored by a Committer.

shipilev · 2023-05-04T09:18:24Z

/sponsor

openjdk · 2023-05-04T09:19:24Z

Going to push as commit 3f1927a.
Since your change was applied there have been 29 commits pushed to the master branch:

e206d57: 8307362: Remove test com/sun/jdi/JdbLastErrorTest.java
82a8e91: 8306489: Open source AWT List related tests
465bdd9: 8307091: A few client tests intermittently throw ConcurrentModificationException
caee1be: 8307305: Update debugger tests to support JTREG_TEST_THREAD_FACTORY mode
3599448: 8307147: [x86] Dangling pointer warning for Assembler::_attributes
64ac9a0: 8306946: jdk/test/lib/process/ProcessToolsStartProcessTest.java fails with "wrong number of lines in OutputAnalyzer output"
03030d4: 8307134: Add GTS root CAs
63cd0a3: 4200096: OffScreenImageSource.removeConsumer NullPointerException
db8b3cd: 8305963: Typo in java.security.Security.getProperty
dcb2f3f: 8306320: BufferedImage spec needs clarification w.r.t its implementation of the WritableRenderedImage interface
... and 19 more: https://git.openjdk.org/jdk/compare/a8bf2acb7db63b508ef169e42a27b9c99178cbb1...master

Your commit was automatically rebased without conflicts.

openjdk · 2023-05-04T09:19:41Z

@shipilev @olivergillespie Pushed as commit 3f1927a.

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

olivergillespie · 2023-05-15T08:24:02Z

/backport jdk17u-dev

openjdk · 2023-05-15T08:24:58Z

@olivergillespie To use the /backport command, you need to be in the OpenJDK census and your GitHub account needs to be linked with your OpenJDK username (how to associate your GitHub account with your OpenJDK username).

olivergillespie · 2023-05-15T13:46:48Z

/backport jdk17u-dev

openjdk · 2023-05-15T13:48:34Z

@olivergillespie Could not automatically backport 3f1927a7 to openjdk/jdk17u-dev due to conflicts in the following files:

src/hotspot/share/gc/g1/g1FullCollector.cpp
src/hotspot/share/gc/parallel/psParallelCompact.cpp

Please fetch the appropriate branch/commit and manually resolve these conflicts by using the following commands in your personal fork of openjdk/jdk17u-dev. Note: these commands are just some suggestions and you can use other equivalent commands you know.

# Fetch the up-to-date version of the target branch
$ git fetch --no-tags https://git.openjdk.org/jdk17u-dev.git master:master

# Check out the target branch and create your own branch to backport
$ git checkout master
$ git checkout -b olivergillespie-backport-3f1927a7

# Fetch the commit you want to backport
$ git fetch --no-tags https://git.openjdk.org/jdk.git 3f1927a7f3a2914402a25335c47a5a8bdd5511a6

# Backport the commit
$ git cherry-pick --no-commit 3f1927a7f3a2914402a25335c47a5a8bdd5511a6
# Resolve conflicts now

# Commit the files you have modified
$ git add files/with/resolved/conflicts
$ git commit -m 'Backport 3f1927a7f3a2914402a25335c47a5a8bdd5511a6'

Once you have resolved the conflicts as explained above continue with creating a pull request towards the openjdk/jdk17u-dev with the title Backport 3f1927a7f3a2914402a25335c47a5a8bdd5511a6.

olivergillespie · 2023-05-16T09:10:38Z

/backport jdk20u

openjdk · 2023-05-16T09:11:59Z

@olivergillespie the backport was successfully created on the branch olivergillespie-backport-3f1927a7 in my personal fork of openjdk/jdk20u. To create a pull request with this backport targeting openjdk/jdk20u:master, just click the following link:

➡️ Create pull request

The title of the pull request is automatically filled in correctly and below you find a suggestion for the pull request body:

Hi all,

This pull request contains a backport of commit 3f1927a7 from the openjdk/jdk repository.

The commit being backported was authored by Oli Gillespie on 4 May 2023 and was reviewed by Thomas Schatzl, Aleksey Shipilev and Albert Mingkun Yang.

Thanks!

If you need to update the source branch of the pull then run the following commands in a local clone of your personal fork of openjdk/jdk20u:

$ git fetch https://github.com/openjdk-bots/jdk20u.git olivergillespie-backport-3f1927a7:olivergillespie-backport-3f1927a7
$ git checkout olivergillespie-backport-3f1927a7
# make changes
$ git add paths/to/changed/files
$ git commit --message 'Describe additional changes made'
$ git push https://github.com/openjdk-bots/jdk20u.git olivergillespie-backport-3f1927a7

8307346 - Add missing gc+phases logging for ObjectCount(AfterGC) JFR …

3ef4f4c

…event collection code

openjdk bot added the rfr Pull request is ready for review label May 3, 2023

openjdk bot added the hotspot-gc hotspot-gc-dev@openjdk.org label May 3, 2023

tschatzl approved these changes May 3, 2023

View reviewed changes

shipilev reviewed May 3, 2023

View reviewed changes

src/hotspot/share/gc/parallel/psParallelCompact.cpp Outdated Show resolved Hide resolved

src/hotspot/share/gc/serial/genMarkSweep.cpp Outdated Show resolved Hide resolved

openjdk bot added the ready Pull request is ready to be integrated label May 3, 2023

Use correct holder var names

ce7227c

shipilev approved these changes May 3, 2023

View reviewed changes

albertnetymk approved these changes May 3, 2023

View reviewed changes

openjdk bot added the sponsor Pull request is ready to be sponsored label May 4, 2023

openjdk bot added the integrated Pull request has been integrated label May 4, 2023

openjdk bot closed this May 4, 2023

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review sponsor Pull request is ready to be sponsored labels May 4, 2023

olivergillespie deleted the 8307346 branch May 4, 2023 13:45

8307346 - Add missing gc+phases logging for ObjectCount(AfterGC) JFR event collection code #13772

8307346 - Add missing gc+phases logging for ObjectCount(AfterGC) JFR event collection code #13772

Conversation

olivergillespie commented May 3, 2023 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewers

Reviewing

Webrev

Uh oh!

bridgekeeper bot commented May 3, 2023

Uh oh!

openjdk bot commented May 3, 2023

Uh oh!

mlbridge bot commented May 3, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

tschatzl left a comment

Choose a reason for hiding this comment

Uh oh!

shipilev left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

openjdk bot commented May 3, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

albertnetymk commented May 3, 2023

Uh oh!

olivergillespie commented May 3, 2023

Uh oh!

shipilev commented May 3, 2023

Uh oh!

shipilev left a comment

Choose a reason for hiding this comment

Uh oh!

olivergillespie commented May 4, 2023

Uh oh!

openjdk bot commented May 4, 2023

Uh oh!

shipilev commented May 4, 2023

Uh oh!

openjdk bot commented May 4, 2023

Uh oh!

openjdk bot commented May 4, 2023

Uh oh!

olivergillespie commented May 15, 2023

Uh oh!

openjdk bot commented May 15, 2023

Uh oh!

olivergillespie commented May 15, 2023

Uh oh!

openjdk bot commented May 15, 2023

Uh oh!

olivergillespie commented May 16, 2023

Uh oh!

openjdk bot commented May 16, 2023

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

olivergillespie commented May 3, 2023 •

edited by openjdk bot

Loading

mlbridge bot commented May 3, 2023 •

edited

Loading

openjdk bot commented May 3, 2023 •

edited

Loading