Skip to content

8376544: [lworld] Use profiling around acmp to speculate that one operand is a value object#2237

Closed
marc-chevalier wants to merge 11 commits intoopenjdk:lworldfrom
marc-chevalier:JDK-8376544
Closed

8376544: [lworld] Use profiling around acmp to speculate that one operand is a value object#2237
marc-chevalier wants to merge 11 commits intoopenjdk:lworldfrom
marc-chevalier:JDK-8376544

Conversation

@marc-chevalier
Copy link
Copy Markdown
Member

@marc-chevalier marc-chevalier commented Mar 17, 2026

To do acmp, if the pointer comparison fails, we check nullity of operands, whether operands have the same class, and if this class is a value class. And if everything works, we call isSubstitutable.

Currently, when we have profiling information, we use it to speculate that operands are null or of an identity class (in which case, pointer comparison is all we should do). This patch proposes to take advantage of the profiling when it hints that operands are value objects. By speculating that at least one operand is of a given value class, the call to isSubstitutable can be intrinsified later, and thus, spare the Java call.

Speculating on one operand is enough since we will check that operands are of the same type.

Some relevant microbenchmark in valhalla.acmp.array.Value032 (in ns/op (lower is better)):

branch_obj_equals000 branch_val_equals000 branch_obj_equals025 branch_val_equals025
Before 4.361 ± 0.153 ns/op 0.907 ± 0.015 ns/op 3.332 ± 0.040 ns/op 0.845 ± 0.032 ns/op
With this fix 0.911 ± 0.019 ns/op 0.899 ± 0.021 ns/op 0.838 ± 0.023 ns/op 0.806 ± 0.012 ns/op
Full results of micro:valhalla.acmp.array.Value032 In more details, before:
Benchmark                                       Mode  Cnt  Score   Error  Units
Value032.branch_obj_equals000                   avgt   15  4.087 ± 0.122  ns/op
Value032.branch_obj_equals025                   avgt   15  3.117 ± 0.067  ns/op
Value032.branch_obj_equals050                   avgt   15  2.235 ± 0.083  ns/op
Value032.branch_obj_equals075                   avgt   15  1.292 ± 0.077  ns/op
Value032.branch_obj_equals100                   avgt   15  0.316 ± 0.004  ns/op
Value032.branch_val_equals000                   avgt   15  0.891 ± 0.024  ns/op
Value032.branch_val_equals025                   avgt   15  0.839 ± 0.033  ns/op
Value032.branch_val_equals050                   avgt   15  0.869 ± 0.087  ns/op
Value032.branch_val_equals075                   avgt   15  0.816 ± 0.023  ns/op
Value032.branch_val_equals100                   avgt   15  0.906 ± 0.032  ns/op
Value032.result_obj_equals000                   avgt   15  4.115 ± 0.185  ns/op
Value032.result_obj_equals025                   avgt   15  3.326 ± 0.346  ns/op
Value032.result_obj_equals050                   avgt   15  2.353 ± 0.516  ns/op
Value032.result_obj_equals075                   avgt   15  1.226 ± 0.040  ns/op
Value032.result_obj_equals100                   avgt   15  0.326 ± 0.006  ns/op
Value032.result_val_equals000                   avgt   15  0.888 ± 0.025  ns/op
Value032.result_val_equals025                   avgt   15  0.824 ± 0.028  ns/op
Value032.result_val_equals050                   avgt   15  0.891 ± 0.039  ns/op
Value032.result_val_equals075                   avgt   15  0.892 ± 0.005  ns/op
Value032.result_val_equals100                   avgt   15  0.908 ± 0.021  ns/op
Value032NullFree.branch_val_equals000           avgt   15  0.186 ± 0.012  ns/op
Value032NullFree.branch_val_equals025           avgt   15  0.461 ± 0.012  ns/op
Value032NullFree.branch_val_equals050           avgt   15  0.468 ± 0.015  ns/op
Value032NullFree.branch_val_equals075           avgt   15  0.465 ± 0.014  ns/op
Value032NullFree.branch_val_equals100           avgt   15  0.189 ± 0.007  ns/op
Value032NullFree.result_val_equals000           avgt   15  0.184 ± 0.004  ns/op
Value032NullFree.result_val_equals025           avgt   15  0.302 ± 0.005  ns/op
Value032NullFree.result_val_equals050           avgt   15  0.448 ± 0.014  ns/op
Value032NullFree.result_val_equals075           avgt   15  0.577 ± 0.014  ns/op
Value032NullFree.result_val_equals100           avgt   15  0.183 ± 0.004  ns/op
Value032NullFreeNonAtomic.branch_val_equals000  avgt   15  0.182 ± 0.006  ns/op
Value032NullFreeNonAtomic.branch_val_equals025  avgt   15  0.470 ± 0.015  ns/op
Value032NullFreeNonAtomic.branch_val_equals050  avgt   15  0.474 ± 0.016  ns/op
Value032NullFreeNonAtomic.branch_val_equals075  avgt   15  0.468 ± 0.017  ns/op
Value032NullFreeNonAtomic.branch_val_equals100  avgt   15  0.188 ± 0.009  ns/op
Value032NullFreeNonAtomic.result_val_equals000  avgt   15  0.190 ± 0.007  ns/op
Value032NullFreeNonAtomic.result_val_equals025  avgt   15  0.322 ± 0.017  ns/op
Value032NullFreeNonAtomic.result_val_equals050  avgt   15  0.473 ± 0.020  ns/op
Value032NullFreeNonAtomic.result_val_equals075  avgt   15  0.592 ± 0.026  ns/op
Value032NullFreeNonAtomic.result_val_equals100  avgt   15  0.189 ± 0.017  ns/op

After:

Benchmark                                       Mode  Cnt  Score   Error  Units
Value032.branch_obj_equals000                   avgt   15  0.967 ± 0.039  ns/op
Value032.branch_obj_equals025                   avgt   15  0.890 ± 0.032  ns/op
Value032.branch_obj_equals050                   avgt   15  0.640 ± 0.019  ns/op
Value032.branch_obj_equals075                   avgt   15  0.466 ± 0.022  ns/op
Value032.branch_obj_equals100                   avgt   15  0.320 ± 0.006  ns/op
Value032.branch_val_equals000                   avgt   15  0.913 ± 0.025  ns/op
Value032.branch_val_equals025                   avgt   15  0.843 ± 0.029  ns/op
Value032.branch_val_equals050                   avgt   15  0.818 ± 0.019  ns/op
Value032.branch_val_equals075                   avgt   15  0.850 ± 0.018  ns/op
Value032.branch_val_equals100                   avgt   15  0.893 ± 0.018  ns/op
Value032.result_obj_equals000                   avgt   15  0.927 ± 0.034  ns/op
Value032.result_obj_equals025                   avgt   15  0.865 ± 0.031  ns/op
Value032.result_obj_equals050                   avgt   15  0.632 ± 0.024  ns/op
Value032.result_obj_equals075                   avgt   15  0.470 ± 0.015  ns/op
Value032.result_obj_equals100                   avgt   15  0.320 ± 0.007  ns/op
Value032.result_val_equals000                   avgt   15  0.889 ± 0.021  ns/op
Value032.result_val_equals025                   avgt   15  0.811 ± 0.023  ns/op
Value032.result_val_equals050                   avgt   15  0.890 ± 0.025  ns/op
Value032.result_val_equals075                   avgt   15  0.923 ± 0.036  ns/op
Value032.result_val_equals100                   avgt   15  0.896 ± 0.027  ns/op
Value032NullFree.branch_val_equals000           avgt   15  0.184 ± 0.006  ns/op
Value032NullFree.branch_val_equals025           avgt   15  0.462 ± 0.015  ns/op
Value032NullFree.branch_val_equals050           avgt   15  0.460 ± 0.027  ns/op
Value032NullFree.branch_val_equals075           avgt   15  0.446 ± 0.014  ns/op
Value032NullFree.branch_val_equals100           avgt   15  0.183 ± 0.006  ns/op
Value032NullFree.result_val_equals000           avgt   15  0.188 ± 0.006  ns/op
Value032NullFree.result_val_equals025           avgt   15  0.317 ± 0.009  ns/op
Value032NullFree.result_val_equals050           avgt   15  0.461 ± 0.012  ns/op
Value032NullFree.result_val_equals075           avgt   15  0.584 ± 0.017  ns/op
Value032NullFree.result_val_equals100           avgt   15  0.181 ± 0.005  ns/op
Value032NullFreeNonAtomic.branch_val_equals000  avgt   15  0.184 ± 0.006  ns/op
Value032NullFreeNonAtomic.branch_val_equals025  avgt   15  0.465 ± 0.019  ns/op
Value032NullFreeNonAtomic.branch_val_equals050  avgt   15  0.451 ± 0.018  ns/op
Value032NullFreeNonAtomic.branch_val_equals075  avgt   15  0.468 ± 0.017  ns/op
Value032NullFreeNonAtomic.branch_val_equals100  avgt   15  0.187 ± 0.009  ns/op
Value032NullFreeNonAtomic.result_val_equals000  avgt   15  0.184 ± 0.007  ns/op
Value032NullFreeNonAtomic.result_val_equals025  avgt   15  0.315 ± 0.011  ns/op
Value032NullFreeNonAtomic.result_val_equals050  avgt   15  0.464 ± 0.021  ns/op
Value032NullFreeNonAtomic.result_val_equals075  avgt   15  0.581 ± 0.018  ns/op
Value032NullFreeNonAtomic.result_val_equals100  avgt   15  0.179 ± 0.002  ns/op

It is same or better.

Tested successfully with tier1-4,stress,valhalla-stress

Thanks,
Marc


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-8376544: [lworld] Use profiling around acmp to speculate that one operand is a value object (Bug - P3)

Reviewers

Contributors

  • Tobias Hartmann <thartmann@openjdk.org>

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 2237

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

Using diff file

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

Using Webrev

Link to Webrev Comment

@marc-chevalier
Copy link
Copy Markdown
Member Author

/contributor add @TobiHartmann

For the first prototype, and significant refactoring.

@bridgekeeper
Copy link
Copy Markdown

bridgekeeper bot commented Mar 17, 2026

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

openjdk bot commented Mar 17, 2026

@marc-chevalier 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:

8376544: [lworld] Use profiling around acmp to speculate that one operand is a value object

Co-authored-by: Tobias Hartmann <thartmann@openjdk.org>
Reviewed-by: thartmann

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 16 new commits pushed to 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.

@openjdk
Copy link
Copy Markdown

openjdk bot commented Mar 17, 2026

@marc-chevalier
Contributor Tobias Hartmann <thartmann@openjdk.org> successfully added.

*/

value class MyValue1NewAcmp {
value class MyValue1MissingOptAcmp {
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

After 8368988: [lworld] Rename classes in valhalla/inlinetypes to be distinct, the class in TestNewAcmp.java was copy-pasted here. I'm just redoing the fix as it makes my IDE complain again.

@marc-chevalier marc-chevalier marked this pull request as ready for review March 17, 2026 10:30
@openjdk openjdk bot added the rfr Pull request is ready for review label Mar 17, 2026
@mlbridge
Copy link
Copy Markdown

mlbridge bot commented Mar 17, 2026

Webrevs

Copy link
Copy Markdown
Member

@TobiHartmann TobiHartmann 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 to me otherwise. Thanks for working on this!

Comment thread test/hotspot/jtreg/compiler/valhalla/inlinetypes/TestLWorldProfiling.java Outdated
@openjdk openjdk bot added the ready Pull request is ready to be integrated label Mar 19, 2026
Copy link
Copy Markdown
Member

@TobiHartmann TobiHartmann left a comment

Choose a reason for hiding this comment

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

Still good. Thanks!

@marc-chevalier
Copy link
Copy Markdown
Member Author

/integrate

Thanks for review!

@openjdk
Copy link
Copy Markdown

openjdk bot commented Mar 20, 2026

Going to push as commit 067e6d8.
Since your change was applied there have been 17 commits pushed to the lworld branch:

Your commit was automatically rebased without conflicts.

@openjdk openjdk bot added the integrated Pull request has been integrated label Mar 20, 2026
@openjdk openjdk bot closed this Mar 20, 2026
@openjdk openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Mar 20, 2026
@openjdk
Copy link
Copy Markdown

openjdk bot commented Mar 20, 2026

@marc-chevalier Pushed as commit 067e6d8.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

integrated Pull request has been integrated

Development

Successfully merging this pull request may close these issues.

2 participants