8333265: De-duplicate method references in java.util.stream.FindOps #19477

cl4es · 2024-05-30T12:50:36Z

Extracting duplicate method references to static field reduce proxy class spinning and loading. In this case 2 less classes loaded when using findAny() on each type of stream.

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-8333265: De-duplicate method references in java.util.stream.FindOps (Enhancement - P5)

Reviewers

Chen Liang (@liach - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 19477

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

Using diff file

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

Webrev

Link to Webrev Comment

bridgekeeper · 2024-05-30T12:51:20Z

👋 Welcome back redestad! 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 · 2024-05-30T12:51:50Z

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

8333265: De-duplicate method references in java.util.stream.FindOps

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

ab8071d: 8338146: Improve Exchanger performance with VirtualThreads
e297e88: 8338688: Shenandoah: Avoid calling java_lang_Class accessors in asserts/verifier
0e8fe35: 8338677: Improve startup of memory access var handles by simplifying combinator chains
3aeb673: 8338532: Speed up the ClassFile API MethodTypeDesc#ofDescriptor
918cf11: 8338490: Serial: Move Generation::print_on to subclasses
80adea8: 8338545: Functional interface implementations for common pre-boot ClassFile operations
7458952: 8338595: Add more linesize for MIME decoder in macro bench test Base64Decode
c4cf1e9: 8338539: New Object to ObjectMonitor mapping: riscv64 implementation
715fa8f: 8336498: [macos] [build]: install-file macro may run into permission denied error
e88a3b0: 8338661: StackMapTable is invalid if frames appear in dead code
... and 970 more: https://git.openjdk.org/jdk/compare/43a2f17342af8f5bf1f5823df9fa0bf0bdfdfce2...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.

openjdk · 2024-05-30T12:52:35Z

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

core-libs

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 · 2024-05-30T12:55:24Z

Webrevs

liach · 2024-05-30T13:04:46Z

Should we extract them to private static utility methods for potential laziness support in the future?

cl4es · 2024-05-30T13:37:01Z

Should we extract them to private static utility methods for potential laziness support in the future?

Ideally we shouldn't have to do any of this. I thought such method references were already de-duplicated in javac - at least within the same compilation units - but I saw this in a startup profile and was surprised myself that the demonstrated manual de-duplication has an observable effect.

liach · 2024-05-30T13:49:57Z

Indeed, CallSites are created per indy instruction instead of per CP indy entry (required by JVMS); thus sharing instruction either in initializers or static methods would have the same effect. javac only deduplicates the CP entry.

forax · 2024-05-30T14:10:39Z

For constant method reference, the solution is to use a constant dynamic instead of an invokedynamic.

cl4es · 2024-05-30T14:30:03Z

Vicente filed a bug on javac to investigate this: https://bugs.openjdk.org/browse/JDK-8333278

I wouldn't mind using condy for constant aka non-capturing lambdas. I recall we had a prototype for that years ago, but switching over was shelved for some reason.

ExE-Boss

The intermediate Predicates and Suppliers don’t need to be fields, and it’s probably better for them to be locals:

src/java.base/share/classes/java/util/stream/FindOps.java

cl4es · 2024-06-18T07:52:44Z

/contributor add @ExE-Boss

openjdk · 2024-06-18T07:52:55Z

@cl4es @ExE-Boss was not found in the census.

Syntax: /contributor (add|remove) [@user | openjdk-user | Full Name <email@address>]. For example:

/contributor add @openjdk-bot
/contributor add duke
/contributor add J. Duke <duke@openjdk.org>

User names can only be used for users in the census associated with this repository. For other contributors you need to supply the full name and email address.

cl4es · 2024-06-26T10:29:58Z

Friendly reminder that this improvement needs a Reviewer. While it'd be great if javac can make this redundant this is adding overhead in the meantime, and the temporary workaround doesn't obfuscate the code.

bridgekeeper · 2024-07-24T14:22:50Z

@cl4es This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

liach · 2024-07-24T14:24:40Z

test/micro/org/openjdk/bench/java/util/stream/ops/ref/FindAny.java

@@ -60,4 +60,10 @@ public Long par_invoke() {
        return LongStream.range(0, size).parallel().boxed().findAny().get();
    }

+    public static void main(String... args) {


Is this driver necessary?

@cl4es Just wonder what your use case is for this addition. If this is accidentally committed, please remove it and I am glad with all other changes.

Adding a main method in micros like these allow me to easily multi-purpose them as relatively clean, diagnostic startup microbenchmarks. It's something I've started adding to all micros I author. I guess I should do a write-up about it some time.

Thanks for the explanation.

bridgekeeper · 2024-08-21T18:57:04Z

@cl4es This pull request has been inactive for more than 8 weeks and will now be automatically closed. If you would like to continue working on this pull request in the future, feel free to reopen it! This can be done using the /open pull request command.

cl4es · 2024-08-21T21:18:26Z

/integrate

openjdk · 2024-08-21T21:19:52Z

@cl4es This pull request has not yet been marked as ready for integration.

cl4es · 2024-08-21T21:24:03Z

/open

openjdk · 2024-08-21T21:24:38Z

@cl4es This pull request is now open

cl4es · 2024-08-21T22:09:43Z

/integrate

openjdk · 2024-08-21T22:10:08Z

Going to push as commit 47c8a6a.
Since your change was applied there have been 980 commits pushed to the master branch:

ab8071d: 8338146: Improve Exchanger performance with VirtualThreads
e297e88: 8338688: Shenandoah: Avoid calling java_lang_Class accessors in asserts/verifier
0e8fe35: 8338677: Improve startup of memory access var handles by simplifying combinator chains
3aeb673: 8338532: Speed up the ClassFile API MethodTypeDesc#ofDescriptor
918cf11: 8338490: Serial: Move Generation::print_on to subclasses
80adea8: 8338545: Functional interface implementations for common pre-boot ClassFile operations
7458952: 8338595: Add more linesize for MIME decoder in macro bench test Base64Decode
c4cf1e9: 8338539: New Object to ObjectMonitor mapping: riscv64 implementation
715fa8f: 8336498: [macos] [build]: install-file macro may run into permission denied error
e88a3b0: 8338661: StackMapTable is invalid if frames appear in dead code
... and 970 more: https://git.openjdk.org/jdk/compare/43a2f17342af8f5bf1f5823df9fa0bf0bdfdfce2...master

Your commit was automatically rebased without conflicts.

openjdk · 2024-08-21T22:10:14Z

@cl4es Pushed as commit 47c8a6a.

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

De-duplicate identical lambdas in FindOps

Verified

This commit was signed with the committer’s verified signature.

IvanGoncharov Ivan Goncharov

GPG key ID: 565A593B3E13C939

Verified
Learn about vigilant mode

Loading
Loading status checks…

cff6d03

openjdk bot added the rfr label May 30, 2024

openjdk bot added the core-libs label May 30, 2024

liach approved these changes May 30, 2024

View reviewed changes

ExE-Boss reviewed Jun 6, 2024

View reviewed changes

@ExE-Boss review

Verified

This commit was signed with the committer’s verified signature.

IvanGoncharov Ivan Goncharov

GPG key ID: 565A593B3E13C939

Verified
Learn about vigilant mode

Loading
Loading status checks…

a223e60

Fixes

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

GPG key ID: 4AEE18F83AFDEB23
Expired

Verified
Learn about vigilant mode

Loading
Loading status checks…

d83a1c9

liach reviewed Jul 24, 2024

View reviewed changes

bridgekeeper bot closed this Aug 21, 2024

openjdk bot reopened this Aug 21, 2024

cl4es requested a review from liach August 21, 2024 21:42

liach approved these changes Aug 21, 2024

View reviewed changes

openjdk bot added the ready label Aug 21, 2024

openjdk bot added the integrated label Aug 21, 2024

openjdk bot closed this Aug 21, 2024

openjdk bot removed ready rfr labels Aug 21, 2024

cl4es deleted the findOps_lambdas branch August 21, 2024 22:13

liach mentioned this pull request Oct 6, 2024

8341594: Use Unsafe to coalesce reads in java.util.zip.ZipUtils #21377

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8333265: De-duplicate method references in java.util.stream.FindOps #19477

8333265: De-duplicate method references in java.util.stream.FindOps #19477

cl4es commented May 30, 2024 •

edited by openjdk bot

Loading

bridgekeeper bot commented May 30, 2024

openjdk bot commented May 30, 2024 •

edited

Loading

openjdk bot commented May 30, 2024

mlbridge bot commented May 30, 2024 •

edited

Loading

liach commented May 30, 2024

cl4es commented May 30, 2024

liach commented May 30, 2024

forax commented May 30, 2024

cl4es commented May 30, 2024

ExE-Boss left a comment

cl4es commented Jun 18, 2024

openjdk bot commented Jun 18, 2024

cl4es commented Jun 26, 2024

bridgekeeper bot commented Jul 24, 2024

liach Jul 24, 2024

liach Aug 21, 2024

cl4es Aug 21, 2024

liach Aug 21, 2024

bridgekeeper bot commented Aug 21, 2024

cl4es commented Aug 21, 2024

openjdk bot commented Aug 21, 2024

cl4es commented Aug 21, 2024

openjdk bot commented Aug 21, 2024

cl4es commented Aug 21, 2024

openjdk bot commented Aug 21, 2024

openjdk bot commented Aug 21, 2024

8333265: De-duplicate method references in java.util.stream.FindOps #19477

8333265: De-duplicate method references in java.util.stream.FindOps #19477

Conversation

cl4es commented May 30, 2024 • edited by openjdk bot Loading

Progress

Issue

Reviewers

Reviewing

Webrev

bridgekeeper bot commented May 30, 2024

openjdk bot commented May 30, 2024 • edited Loading

openjdk bot commented May 30, 2024

mlbridge bot commented May 30, 2024 • edited Loading

Webrevs

liach commented May 30, 2024

cl4es commented May 30, 2024

liach commented May 30, 2024

forax commented May 30, 2024

cl4es commented May 30, 2024

ExE-Boss left a comment

Choose a reason for hiding this comment

cl4es commented Jun 18, 2024

openjdk bot commented Jun 18, 2024

cl4es commented Jun 26, 2024

bridgekeeper bot commented Jul 24, 2024

liach Jul 24, 2024

Choose a reason for hiding this comment

liach Aug 21, 2024

Choose a reason for hiding this comment

cl4es Aug 21, 2024

Choose a reason for hiding this comment

liach Aug 21, 2024

Choose a reason for hiding this comment

bridgekeeper bot commented Aug 21, 2024

cl4es commented Aug 21, 2024

openjdk bot commented Aug 21, 2024

cl4es commented Aug 21, 2024

openjdk bot commented Aug 21, 2024

cl4es commented Aug 21, 2024

openjdk bot commented Aug 21, 2024

openjdk bot commented Aug 21, 2024

cl4es commented May 30, 2024 •

edited by openjdk bot

Loading

openjdk bot commented May 30, 2024 •

edited

Loading

mlbridge bot commented May 30, 2024 •

edited

Loading