8383559: [lworld] Autobox cache removal causes performance regression with Renaissance benchmarks by TobiHartmann · Pull Request #2390 · openjdk/valhalla

TobiHartmann · 2026-05-05T14:25:21Z

To re-enable the autobox cache, this patch reverts (parts of) JDK-8369921, JDK-8378476, deopt parts of JDK-8378531 and JDK-8379148.

I had to modify the newCacheArray methods to return a non-flattened array to avoid buffering.

I wasn't sure about the comments above the valueOf methods:
b3ccb6a#diff-6136f7a80110ec00bf9a0b7da9943eb2fc33025ab400bc1f70153c1b2ac36993R146

Should they be reverted too? For now, I left them as is.

Thanks,
Tobias

I confirm that I make this contribution in accordance with the OpenJDK Interim AI Policy.

Progress

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

Issue

JDK-8383559: [lworld] Autobox cache removal causes performance regression with Renaissance benchmarks (Bug - P2)

Reviewers

Ioi Lam (@iklam - Committer)
Matias Saavedra Silva (@matias9927 - Committer)
Chen Liang (@liach - Committer)

Contributors

Ioi Lam <iklam@openjdk.org>

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 2390

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/valhalla/pull/2390.diff

Using Webrev

Link to Webrev Comment

bridgekeeper · 2026-05-05T14:26:59Z

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

openjdk · 2026-05-05T14:28:20Z

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

8383559: [lworld] Autobox cache removal causes performance regression with Renaissance benchmarks

Co-authored-by: Ioi Lam <iklam@openjdk.org>
Reviewed-by: iklam, matsaave, liach

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 3 new commits pushed to the lworld branch:

6b9eb7d: 8384283: [lworld] Post-parse call devirtualization with unloaded return type fails
de6aff4: 8384364: [lworld] C2: assert(cloned_flat_array_check->req() == 3) failed: unexpected number of inputs for FlatArrayCheck
2d330b1: 8384193: [lworld] Remove identity field from java.lang.Class

Please see this link for an up-to-date comparison between the source branch of this pull request and the lworld 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 lworld branch, type /integrate in a new comment.

mlbridge · 2026-05-05T15:19:33Z

Webrevs

Arraying

The code changes look good overall, thanks! Maybe @matias9927 knows more about the AOT-specifics here and can weigh in as well.

I don't really have the expertise on specification to weigh in, but it seems like Victor has already pointed out some mistakes.

liach · 2026-05-06T12:30:29Z

+            assert cache.length == size;
+        }
+
+        private static Byte[] newCacheArray(int size) {


I don't think we need this - we should be able to revert to mainline code. The only code I think that need any update would be the old new Byte[size] should become (Byte[]) ValueClass.newReferenceArray(Byte.class, size) both with and without preview.

new Byte[size] should become (Byte[]) ValueClass.newReferenceArray(Byte.class, size) both with and without preview.

@liach That's not possible because ValueClass.newReferenceArray throws when preview is disabled because boxes are identity classes then.

liach · 2026-05-06T15:28:43Z

For JDK-8383894, I think we need to move the @DeserializeConstructor annotations from the valueOf methods to the constructors to eliminate that particular regression. Can you include that in your patch?

TobiHartmann · 2026-05-08T14:09:08Z

Thanks for the reviews! I've been working on https://git.openjdk.org/valhalla/pull/2406 which was blocking this PR and will get to this next week.

TobiHartmann · 2026-05-13T10:52:19Z

I think we need to move the @DeserializeConstructor annotations from the valueOf methods to the constructors to eliminate that particular regression. Can you include that in your patch?

Done. Maybe @kuksenko could verify that this solves JDK-8383894.

TobiHartmann · 2026-05-13T10:52:29Z

/contributor add @iklam

openjdk · 2026-05-13T10:53:29Z

@TobiHartmann
Contributor Ioi Lam <iklam@openjdk.org> successfully added.

TobiHartmann · 2026-05-13T10:56:00Z

Thanks for the reviews! I addressed all comments and will re-run testing.

viktorklang-ora · 2026-05-13T12:16:04Z

@@ -25,7 +25,6 @@
 /*
 * @test
 * @summary Test primitive box caches integrity in various scenarios (IntegerCache etc)
- * @requires !java.enablePreview


Are we sure that there are no other tests that need their requires-clauses updated as a part of this change?

I had a quick look and nothing else stood out. But I defer to people more familiar in this area.

TobiHartmann · 2026-05-13T13:26:26Z

If there are no strong objections, I propose to integrate this as is because some other work depends on it. I would leave it to the core-libs / CDS experts to follow-up with more cleanups.

iklam

CDS changes look OK.

matias9927

CDS changes look good!

liach

Core libs updates look good.

TobiHartmann · 2026-05-13T14:32:54Z

Thanks for the reviews. I'll wait for testing to all pass before integrating (probably only on Friday because Thursday is a public holiday here).

TobiHartmann · 2026-05-15T06:16:07Z

/integrate

openjdk · 2026-05-15T06:17:42Z

Going to push as commit 50ed255.
Since your change was applied there have been 6 commits pushed to the lworld branch:

37504c6: 8384202: [lworld] C2: assert(offset == field->offset_in_bytes()) failed: offset mismatch
4824ac0: 8383164: [lworld] javadoc updates for value class serialization
cc40808: 8384105: [lworld] serviceability/jvmti/valhalla/SampledObjectAllocValue/SampledObjectAllocValue.java fails after JDK-8382702
... and 3 more: https://git.openjdk.org/valhalla/compare/7c2d2cd3d559577a7c3507b2ee814062eed72c06...lworld

Your commit was automatically rebased without conflicts.

openjdk · 2026-05-15T06:17:53Z

@TobiHartmann Pushed as commit 50ed255.

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

JDK-8383559

f24c55e

TobiHartmann marked this pull request as ready for review May 5, 2026 15:13

openjdk Bot added the rfr Pull request is ready for review label May 5, 2026

TobiHartmann commented May 6, 2026

View reviewed changes

Comment thread src/hotspot/share/opto/type.cpp Outdated

Update src/hotspot/share/opto/type.cpp

88b9e2f

viktorklang-ora reviewed May 6, 2026

View reviewed changes

Comment thread src/java.base/share/classes/java/lang/Short.java

Arraying reviewed May 6, 2026

View reviewed changes

Comment thread src/hotspot/share/runtime/deoptimization.cpp

Comment thread src/java.base/share/classes/java/lang/Character.java Outdated

Comment thread src/java.base/share/classes/java/lang/Integer.java Outdated

liach reviewed May 6, 2026

View reviewed changes

viktorklang-ora reviewed May 6, 2026

View reviewed changes

Comment thread src/java.base/share/classes/java/lang/Byte.java Outdated

TobiHartmann mentioned this pull request May 7, 2026

8384052: [lworld] Default refined klass invariant in ObjArrayKlass::klass_from_description is broken #2406

Closed

4 tasks

Reviewer comments

d51e0ad

Merge branch 'lworld' into JDK-8383559

94f0f66

viktorklang-ora reviewed May 13, 2026

View reviewed changes

iklam approved these changes May 13, 2026

View reviewed changes

openjdk Bot added the ready Pull request is ready to be integrated label May 13, 2026

matias9927 approved these changes May 13, 2026

View reviewed changes

liach approved these changes May 13, 2026

View reviewed changes

openjdk Bot added the integrated Pull request has been integrated label May 15, 2026

openjdk Bot closed this May 15, 2026

openjdk Bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels May 15, 2026

Conversation

TobiHartmann commented May 5, 2026 • edited by openjdk Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewers

Contributors

Reviewing

Uh oh!

bridgekeeper Bot commented May 5, 2026

Uh oh!

openjdk Bot commented May 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mlbridge Bot commented May 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

Uh oh!

Uh oh!

Arraying left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

liach May 6, 2026

Choose a reason for hiding this comment

Uh oh!

TobiHartmann May 13, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

liach commented May 6, 2026

Uh oh!

TobiHartmann commented May 8, 2026

Uh oh!

TobiHartmann commented May 13, 2026

Uh oh!

TobiHartmann commented May 13, 2026

Uh oh!

openjdk Bot commented May 13, 2026

Uh oh!

TobiHartmann commented May 13, 2026

Uh oh!

viktorklang-ora May 13, 2026

Choose a reason for hiding this comment

Uh oh!

TobiHartmann May 13, 2026

Choose a reason for hiding this comment

Uh oh!

TobiHartmann commented May 13, 2026

Uh oh!

iklam left a comment

Choose a reason for hiding this comment

Uh oh!

matias9927 left a comment

Choose a reason for hiding this comment

Uh oh!

liach left a comment

Choose a reason for hiding this comment

Uh oh!

TobiHartmann commented May 13, 2026

Uh oh!

TobiHartmann commented May 15, 2026

Uh oh!

openjdk Bot commented May 15, 2026

Uh oh!

openjdk Bot commented May 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

6 participants

TobiHartmann commented May 5, 2026 •

edited by openjdk Bot

Loading

openjdk Bot commented May 5, 2026 •

edited

Loading

mlbridge Bot commented May 5, 2026 •

edited

Loading