8250961: Move Universe::update_heap_info_at_gc to CollectedHeap #27

adityamandaleeka · 2020-09-06T07:00:36Z

This is a small refactoring that moves update_heap_info_at_gc() and related members to CollectedHeap. There should be no behavioral change.

jdk/hotspot tier1 passed

JBS: https://bugs.openjdk.java.net/browse/JDK-8250961

Progress

Change must not contain extraneous whitespace
Commit message must refer to an issue
Change must be properly reviewed

Issue

JDK-8250961: Move Universe::update_heap_info_at_gc to CollectedHeap

Reviewers

Stefan Karlsson (@stefank - Reviewer) ⚠️ Review applies to 64d1c39
Kim Barrett (@kimbarrett - Reviewer)

Download

$ git fetch https://git.openjdk.java.net/jdk pull/27/head:pull/27
$ git checkout pull/27

bridgekeeper · 2020-09-06T07:02:11Z

👋 Welcome back adityam! 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 · 2020-09-06T07:03:08Z

@adityamandaleeka The following labels will be automatically applied to this pull request: hotspot`, `shenandoah. When this pull request is ready to be reviewed, an RFR email will be sent to the corresponding mailing lists. If you would like to change these labels, use the /label (add|remove) "label" command.

adityamandaleeka · 2020-09-06T07:05:02Z

/label add "hotspot-gc"

openjdk · 2020-09-06T07:05:25Z

@adityamandaleeka Usage: /label <add|remove> [label[, label, ...]]wherelabel` is an additional classification that should be applied to this PR. These labels are valid:

serviceability
hotspot
sound
hotspot-compiler
kulla
i18n
shenandoah
jdk
javadoc
2d
security
swing
hotspot-runtime
jmx
build
nio
beans
core-libs
compiler
net
hotspot-gc
hotspot-jfr
awt

adityamandaleeka · 2020-09-06T07:06:51Z

/label add hotspot-gc

openjdk · 2020-09-06T07:07:01Z

@adityamandaleeka
The hotspot-gc label was successfully added.

adityamandaleeka · 2020-09-06T07:07:57Z

/label remove hotspot

openjdk · 2020-09-06T07:08:04Z

@adityamandaleeka
The hotspot label was successfully removed.

mlbridge · 2020-09-06T07:10:53Z

Webrevs

stefank · 2020-09-07T08:25:55Z

src/hotspot/share/gc/shared/collectedHeap.hpp

+  // Historic gc information
+  size_t _heap_capacity_at_last_gc;
+  size_t _heap_used_at_last_gc;
+


These should be initialized in the constructor.

Move these up to the other variables.

stefank

Now that the variables and functions have moved to CollectedHeap, I don't think we need the heap infix anymore.

Maybe rename variables to be named:
_capacity_at_last_gc
_used_at_last_gc

Functions:
free_at_last_gc()
used_at_last_gc()
update_capacity_and_used_at_gc()

stefank · 2020-09-07T08:32:56Z

src/hotspot/share/gc/shared/collectedHeap.hpp

+  // Historic gc information
+  size_t get_heap_free_at_last_gc()    { return _heap_capacity_at_last_gc - _heap_used_at_last_gc; }
+  size_t get_heap_used_at_last_gc()    { return _heap_used_at_last_gc; }
+  void update_heap_info_at_gc();
+


Maybe move these to after unused(). Alternatively, move it near soft_ref_policy(), since this is currently only used by soft ref handling.

stefank · 2020-09-07T08:34:19Z

/label remove shenandoah

openjdk · 2020-09-07T08:34:38Z

@stefank
The shenandoah label was successfully removed.

adityamandaleeka · 2020-09-08T23:52:48Z

@stefank Thanks for the suggestions. I've updated the PR.

stefank

Thanks. Just two small nits below. You still need a second reviewer before this can be integrated.

stefank · 2020-09-09T06:40:50Z

src/hotspot/share/gc/shared/collectedHeap.hpp

+  size_t free_at_last_gc()    { return _capacity_at_last_gc - _used_at_last_gc; }
+  size_t used_at_last_gc()    { return _used_at_last_gc; }


There are some unnecessary spaces between () and {. Could you also make these functions const?

openjdk · 2020-09-09T06:44:41Z

@adityamandaleeka This change now passes all automated pre-integration checks. In addition to the automated checks, the change must also fulfill all project specific requirements

After integration, the commit message will be:

8250961: Move Universe::update_heap_info_at_gc to CollectedHeap

Reviewed-by: stefank, kbarrett

If you would like to add a summary, use the /summary command.
To credit additional contributors, use the /contributor command.
To add additional solved issues, use the /issue command.

Since the source branch of this PR was last updated there have been 34 commits pushed to the master branch:

f78f780: 8252889: Obsolete -XX:+InsertMemBarAfterArraycopy
f933961: 8252980: comment only changes extracted from JDK-8247281
4333942: 8250217: com.sun.tools.javac.api.JavacTaskImpl swallows compiler exceptions potentially producing false positive test results
5166094: 8252957: Wrong comment in CgroupV1Subsystem::cpu_quota
6329de4: 8248532: Every time I change keyboard language at my MacBook, Java crashes
d560964: 8252794: Creation of JNIMethodBlock should be done with a leaf lock
5fef8dd: 8235229: Compilation against a modular, multi-release JAR erroneous with --release
382b8fe: 8240751: Shenandoah: fold ShenandoahTracer definition
c98417e: 8250840: some tests use --enable-preview unnecessarily
c655b70: 8252916: Newline in object field values list of ScopeDesc should be removed
... and 24 more: https://git.openjdk.java.net/jdk/compare/1262ae36af0c9a41609765966123751b5ad9098b...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid automatic rebasing, please merge master into your branch, and then specify the current head hash when integrating, like this: /integrate f78f7805707f49033530acc66ba4c625847c2918.

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 (@stefank, @tschatzl, @kimbarrett) 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).

stefank · 2020-09-09T06:46:52Z

/reviewers 2

openjdk · 2020-09-09T06:47:09Z

@stefank
The number of required reviews for this PR is now set to 2 (with at least 1 of role reviewers).

kimbarrett · 2020-09-09T10:58:19Z

Looks good except for Stefan's comments about free/used_at_last_gc().

openjdk · 2020-09-09T12:58:14Z

@kimbarrett Only the author (@adityamandaleeka) is allowed to issue the reviewer command.

kimbarrett

I'm assuming the small changes to free/used_at_last_gc will be made.

adityamandaleeka · 2020-09-10T00:02:27Z

Thanks for the reviews @stefank @tschatzl @kimbarrett . I pushed an update addressing the last comments.

stefank · 2020-09-10T05:15:53Z

Thanks. Could you merge in master, make sure that this still builds, and run the jtreg tests in test/hotspot/jtreg/gc? After that you can add a comment with /integrate, and I'll sponsor this patch for you.

adityamandaleeka · 2020-09-10T06:47:09Z

/integrate

openjdk · 2020-09-10T06:48:06Z

@adityamandaleeka
Your change (at version 59b0c8d) is now ready to be sponsored by a Committer.

stefank · 2020-09-10T07:00:38Z

@adityamandaleeka Your branch is still 34 commits behind openjdk:master. It doesn't look like you merged the master branch into your 8250961. Or did you only do that locally?

adityamandaleeka · 2020-09-10T07:06:34Z

@stefank Yes, I did the merge/build/test locally and didn't push anything since I saw the comment from the bot above saying that the change would be auto-rebased when merging since there are no conflicts with my branch.

openjdk · 2020-09-10T07:08:23Z

@stefank Only the author (@adityamandaleeka) is allowed to issue the integrate command. As this PR is ready to be sponsored, and you are an eligible sponsor, did you mean to issue the /sponsor command?

stefank · 2020-09-10T07:08:31Z

/sponsor

openjdk · 2020-09-10T07:09:33Z

@stefank @adityamandaleeka Since your change was applied there have been 34 commits pushed to the master branch:

f78f780: 8252889: Obsolete -XX:+InsertMemBarAfterArraycopy
f933961: 8252980: comment only changes extracted from JDK-8247281
4333942: 8250217: com.sun.tools.javac.api.JavacTaskImpl swallows compiler exceptions potentially producing false positive test results
5166094: 8252957: Wrong comment in CgroupV1Subsystem::cpu_quota
6329de4: 8248532: Every time I change keyboard language at my MacBook, Java crashes
d560964: 8252794: Creation of JNIMethodBlock should be done with a leaf lock
5fef8dd: 8235229: Compilation against a modular, multi-release JAR erroneous with --release
382b8fe: 8240751: Shenandoah: fold ShenandoahTracer definition
c98417e: 8250840: some tests use --enable-preview unnecessarily
c655b70: 8252916: Newline in object field values list of ScopeDesc should be removed
... and 24 more: https://git.openjdk.java.net/jdk/compare/1262ae36af0c9a41609765966123751b5ad9098b...master

Your commit was automatically rebased without conflicts.

Pushed as commit 6a00534.

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

stefank · 2020-09-10T07:46:51Z

This caused a failure building without precompiled headers. @kimbarrett is handling that with JDK-8252995.

…cv-port-25 RISC-V: loom: Adjust loom's bottom frame to RISC-V frame's abi

@franferrax

… jtreg main class (openjdk#27) Reviewed-by: @franferrax, @gnu-andrew

JVM-1874: materialization breaks the trailing barrier in Initializer

@franferrax

… jtreg main class (openjdk#27) Reviewed-by: @franferrax, @gnu-andrew

Fix matcher and access flags

Reviewed-by: asmehra

8250961: Move Universe::update_heap_info_at_gc to CollectedHeap

0263ed9

openjdk bot added the rfr Pull request is ready for review label Sep 6, 2020

openjdk bot added hotspot hotspot-dev@openjdk.org shenandoah shenandoah-dev@openjdk.org labels Sep 6, 2020

openjdk bot added the hotspot-gc hotspot-gc-dev@openjdk.org label Sep 6, 2020

openjdk bot removed the hotspot hotspot-dev@openjdk.org label Sep 6, 2020

stefank reviewed Sep 7, 2020

View reviewed changes

openjdk bot removed the shenandoah shenandoah-dev@openjdk.org label Sep 7, 2020

Rename and rearrange things a bit per feedback.

64d1c39

stefank approved these changes Sep 9, 2020

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Sep 9, 2020

openjdk bot removed the ready Pull request is ready to be integrated label Sep 9, 2020

kimbarrett approved these changes Sep 9, 2020

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Sep 9, 2020

Fix spacing and make functions const.

59b0c8d

kimbarrett approved these changes Sep 10, 2020

View reviewed changes

openjdk bot added the sponsor Pull request is ready to be sponsored label Sep 10, 2020

openjdk bot closed this Sep 10, 2020

openjdk bot added integrated Pull request has been integrated and removed sponsor Pull request is ready to be sponsored ready Pull request is ready to be integrated rfr Pull request is ready for review labels Sep 10, 2020

manukumarvs mentioned this pull request Oct 1, 2021

8274597: [TESTBUG] Two of the dnd tests times out and fails #5777

Closed

3 tasks

DamonFool mentioned this pull request May 3, 2022

8286066: assert(k != __null) failed: klass not loaded caused by FillerObject_klass #8519

Closed

3 tasks

zhengxiaolinX pushed a commit to zhengxiaolinX/jdk that referenced this pull request Sep 22, 2022

Merge pull request openjdk#27 from zhengxiaolinX/JDK-8286301-loom-ris…

57debca

…cv-port-25 RISC-V: loom: Adjust loom's bottom frame to RISC-V frame's abi

gnu-andrew pushed a commit to gnu-andrew/jdk that referenced this pull request Apr 12, 2023

test/jdk/sun/security/pkcs11/fips/VerifyMissingAttributes.java: fixed…

265e3df

… jtreg main class (openjdk#27) Reviewed-by: @franferrax, @gnu-andrew

caojoshua pushed a commit to caojoshua/jdk that referenced this pull request May 8, 2023

Merge pull request openjdk#27 from navyxliu/PEA_parser

64f136c

JVM-1874: materialization breaks the trailing barrier in Initializer

gnu-andrew pushed a commit to gnu-andrew/jdk that referenced this pull request Aug 18, 2023

test/jdk/sun/security/pkcs11/fips/VerifyMissingAttributes.java: fixed…

50ca5b9

… jtreg main class (openjdk#27) Reviewed-by: @franferrax, @gnu-andrew

calvinccheung mentioned this pull request Mar 25, 2025

8352579: Refactor CDS legacy optimization for lambda proxy classes #24145

Closed

3 tasks

biboudis added a commit to biboudis/jdk that referenced this pull request May 8, 2025

Merge pull request openjdk#27 from liach/fix/matcher-af

737edc2

Fix matcher and access flags

iklam pushed a commit to iklam/jdk that referenced this pull request Jun 24, 2025

Missing nmethod preload bit after openjdk#27

15d55e5

Reviewed-by: asmehra

		size_t free_at_last_gc() { return _capacity_at_last_gc - _used_at_last_gc; }
		size_t used_at_last_gc() { return _used_at_last_gc; }

8250961: Move Universe::update_heap_info_at_gc to CollectedHeap #27

8250961: Move Universe::update_heap_info_at_gc to CollectedHeap #27

Uh oh!

Conversation

adityamandaleeka commented Sep 6, 2020 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewers

Download

Uh oh!

bridgekeeper bot commented Sep 6, 2020

Uh oh!

openjdk bot commented Sep 6, 2020

Uh oh!

adityamandaleeka commented Sep 6, 2020

Uh oh!

openjdk bot commented Sep 6, 2020

Uh oh!

adityamandaleeka commented Sep 6, 2020

Uh oh!

openjdk bot commented Sep 6, 2020

Uh oh!

adityamandaleeka commented Sep 6, 2020

Uh oh!

openjdk bot commented Sep 6, 2020

Uh oh!

mlbridge bot commented Sep 6, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

stefank Sep 7, 2020

Choose a reason for hiding this comment

Uh oh!

stefank Sep 7, 2020

Choose a reason for hiding this comment

Uh oh!

stefank left a comment

Choose a reason for hiding this comment

Uh oh!

stefank Sep 7, 2020

Choose a reason for hiding this comment

Uh oh!

stefank commented Sep 7, 2020

Uh oh!

openjdk bot commented Sep 7, 2020

Uh oh!

adityamandaleeka commented Sep 8, 2020

Uh oh!

stefank left a comment

Choose a reason for hiding this comment

Uh oh!

stefank Sep 9, 2020

Choose a reason for hiding this comment

Uh oh!

openjdk bot commented Sep 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stefank commented Sep 9, 2020

Uh oh!

openjdk bot commented Sep 9, 2020

Uh oh!

kimbarrett commented Sep 9, 2020

Uh oh!

openjdk bot commented Sep 9, 2020

Uh oh!

kimbarrett left a comment

Choose a reason for hiding this comment

Uh oh!

adityamandaleeka commented Sep 10, 2020

Uh oh!

stefank commented Sep 10, 2020

Uh oh!

adityamandaleeka commented Sep 10, 2020

Uh oh!

openjdk bot commented Sep 10, 2020

Uh oh!

stefank commented Sep 10, 2020

Uh oh!

adityamandaleeka commented Sep 10, 2020

adityamandaleeka commented Sep 6, 2020 •

edited by openjdk bot

Loading

mlbridge bot commented Sep 6, 2020 •

edited

Loading

openjdk bot commented Sep 9, 2020 •

edited

Loading