8271008: appcds/*/MethodHandlesAsCollectorTest.java tests time out because of excessive GC (CodeCache GC Threshold) in loom #7514

coleenp · 2022-02-17T13:34:39Z

In Loom there's a full heap walk when the sweeper is triggered. Many of the triggers in this test case are for the adapters created by the test, which are not deallocated. Since there is a fall back to other code cache heap areas for NonNMethod and for NMethodProfiled, made the function CodeCache::reverse_free_ratio() examine the total code cache available rather than the specific area that it is allocating into. The compilation policy also uses this to increase the C1 compile threshold so also uses the entire free code cache size to calculate new threshold (ask @TobiHartmann about this). Thanks to Tobias for the discussion for this fix.
Tested with tier1-4.

Progress

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

Issue

JDK-8271008: appcds/*/MethodHandlesAsCollectorTest.java tests time out because of excessive GC (CodeCache GC Threshold) in loom

Reviewers

Tobias Hartmann (@TobiHartmann - Reviewer) ⚠️ Review applies to 7a69dc4
Erik Österlund (@fisk - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk pull/7514/head:pull/7514
$ git checkout pull/7514

Update a local copy of the PR:
$ git checkout pull/7514
$ git pull https://git.openjdk.java.net/jdk pull/7514/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 7514

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jdk/pull/7514.diff

bridgekeeper · 2022-02-17T13:36:13Z

👋 Welcome back coleenp! 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 · 2022-02-17T13:37:39Z

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

hotspot-compiler

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.

…cause of excessive GC (CodeCache GC Threshold) in loom

mlbridge · 2022-02-18T20:36:18Z

Webrevs

01: Full - Incremental (97b7a59c)
00: Full (7a69dc43)

coleenp · 2022-02-18T22:17:34Z

/label add hotspot

openjdk · 2022-02-18T22:18:36Z

@coleenp
The hotspot label was successfully added.

coleenp · 2022-02-18T22:18:52Z

/label remove hotspot-compiler

openjdk · 2022-02-18T22:19:35Z

@coleenp
The hotspot-compiler label was successfully removed.

TobiHartmann

Looks good to me. Thanks for fixing this.

TobiHartmann · 2022-02-21T09:15:05Z

src/hotspot/share/code/codeCache.cpp

+// Returns the reverse free ratio. E.g., if 25% (1/4) of the code heap
+// is free, reverse_free_ratio() returns 4.
+// Since code heap for each type of code blobs falls forward to the next
+// type of code heap, return the reverse free ratio for the entire
+// code heap.


Suggested change

// Returns the reverse free ratio. E.g., if 25% (1/4) of the code heap

// is free, reverse_free_ratio() returns 4.

// Since code heap for each type of code blobs falls forward to the next

// type of code heap, return the reverse free ratio for the entire

// code heap.

// Returns the reverse free ratio. E.g., if 25% (1/4) of the code cache

// is free, reverse_free_ratio() returns 4.

// Since code heap for each type of code blobs falls forward to the next

// type of code heap, return the reverse free ratio for the entire

// code cache.

openjdk · 2022-02-21T09:18:20Z

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

8271008: appcds/*/MethodHandlesAsCollectorTest.java tests time out because of excessive GC (CodeCache GC Threshold) in loom

Reviewed-by: thartmann, 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 23 new commits pushed to the master branch:

ab6d8e6: 8260328: Drop redundant CSS properties from java.desktop HTML files
b95310b: 8282220: contentType should not be a PKCS7's member
bc43320: 8281543: Remove unused code/headerfile dtraceAttacher.hpp
f953952: 8281745: Create a regression test for JDK-4514331
e0b4962: 8282190: Typo in javadoc of java.time.format.DateTimeFormatter#getDecimalStyle
e1c98bd: 8281523: Accessibility: Conversion from string literal loses const qualifier
cc7cf81: 8280861: Robot color picker broken on Linux with scaling above 100%
d7a706a: 8253757: Add LLVM-based backend for hsdis
bdae1d8: 8282147: [TESTBUG] waitForIdle after creating frame in JSpinnerMouseAndKeyPressTest.java
51f4420: 8282130: (bf) Remove unused ARRAY_BASE_OFFSET, ARRAY_INDEX_SCALE from read-only Heap Buffers
... and 13 more: https://git.openjdk.java.net/jdk/compare/f5120b764c6f84776e7ea335d7ff59b16f6496b0...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.

coleenp · 2022-02-21T15:07:17Z

Thanks, Tobias.

fisk

Looks good.

coleenp · 2022-02-22T13:42:29Z

Thanks Erik!
/integrate

openjdk · 2022-02-22T13:43:09Z

Going to push as commit 022d807.
Since your change was applied there have been 23 commits pushed to the master branch:

ab6d8e6: 8260328: Drop redundant CSS properties from java.desktop HTML files
b95310b: 8282220: contentType should not be a PKCS7's member
bc43320: 8281543: Remove unused code/headerfile dtraceAttacher.hpp
f953952: 8281745: Create a regression test for JDK-4514331
e0b4962: 8282190: Typo in javadoc of java.time.format.DateTimeFormatter#getDecimalStyle
e1c98bd: 8281523: Accessibility: Conversion from string literal loses const qualifier
cc7cf81: 8280861: Robot color picker broken on Linux with scaling above 100%
d7a706a: 8253757: Add LLVM-based backend for hsdis
bdae1d8: 8282147: [TESTBUG] waitForIdle after creating frame in JSpinnerMouseAndKeyPressTest.java
51f4420: 8282130: (bf) Remove unused ARRAY_BASE_OFFSET, ARRAY_INDEX_SCALE from read-only Heap Buffers
... and 13 more: https://git.openjdk.java.net/jdk/compare/f5120b764c6f84776e7ea335d7ff59b16f6496b0...master

Your commit was automatically rebased without conflicts.

openjdk · 2022-02-22T13:43:29Z

@coleenp Pushed as commit 022d807.

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

openjdk bot added the hotspot-compiler hotspot-compiler-dev@openjdk.org label Feb 17, 2022

8271008: appcds/*/MethodHandlesAsCollectorTest.java tests time out be…

7a69dc4

…cause of excessive GC (CodeCache GC Threshold) in loom

coleenp force-pushed the reverse branch from 7e27c26 to 7a69dc4 Compare February 18, 2022 20:18

coleenp marked this pull request as ready for review February 18, 2022 20:31

openjdk bot added the rfr Pull request is ready for review label Feb 18, 2022

openjdk bot added the hotspot hotspot-dev@openjdk.org label Feb 18, 2022

openjdk bot removed the hotspot-compiler hotspot-compiler-dev@openjdk.org label Feb 18, 2022

TobiHartmann approved these changes Feb 21, 2022

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Feb 21, 2022

Fixed comment

97b7a59

fisk approved these changes Feb 22, 2022

View reviewed changes

openjdk bot added the integrated Pull request has been integrated label Feb 22, 2022

openjdk bot closed this Feb 22, 2022

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Feb 22, 2022

coleenp deleted the reverse branch February 22, 2022 14:00

8271008: appcds/*/MethodHandlesAsCollectorTest.java tests time out because of excessive GC (CodeCache GC Threshold) in loom #7514

8271008: appcds/*/MethodHandlesAsCollectorTest.java tests time out because of excessive GC (CodeCache GC Threshold) in loom #7514

coleenp commented Feb 17, 2022 •

edited by openjdk bot

bridgekeeper bot commented Feb 17, 2022

openjdk bot commented Feb 17, 2022

mlbridge bot commented Feb 18, 2022 •

edited

coleenp commented Feb 18, 2022

openjdk bot commented Feb 18, 2022

coleenp commented Feb 18, 2022

openjdk bot commented Feb 18, 2022

TobiHartmann left a comment

TobiHartmann Feb 21, 2022

coleenp Feb 21, 2022

openjdk bot commented Feb 21, 2022 •

edited

coleenp commented Feb 21, 2022

fisk left a comment

coleenp commented Feb 22, 2022

openjdk bot commented Feb 22, 2022

openjdk bot commented Feb 22, 2022

8271008: appcds/*/MethodHandlesAsCollectorTest.java tests time out because of excessive GC (CodeCache GC Threshold) in loom #7514

8271008: appcds/*/MethodHandlesAsCollectorTest.java tests time out because of excessive GC (CodeCache GC Threshold) in loom #7514

Conversation

coleenp commented Feb 17, 2022 • edited by openjdk bot

Progress

Issue

Reviewers

Reviewing

bridgekeeper bot commented Feb 17, 2022

openjdk bot commented Feb 17, 2022

mlbridge bot commented Feb 18, 2022 • edited

Webrevs

coleenp commented Feb 18, 2022

openjdk bot commented Feb 18, 2022

coleenp commented Feb 18, 2022

openjdk bot commented Feb 18, 2022

TobiHartmann left a comment

Choose a reason for hiding this comment

TobiHartmann Feb 21, 2022

Choose a reason for hiding this comment

coleenp Feb 21, 2022

Choose a reason for hiding this comment

openjdk bot commented Feb 21, 2022 • edited

coleenp commented Feb 21, 2022

fisk left a comment

Choose a reason for hiding this comment

coleenp commented Feb 22, 2022

openjdk bot commented Feb 22, 2022

openjdk bot commented Feb 22, 2022

coleenp commented Feb 17, 2022 •

edited by openjdk bot

mlbridge bot commented Feb 18, 2022 •

edited

openjdk bot commented Feb 21, 2022 •

edited