jdk
jdk copied to clipboard
openjdk
8338544: Dedicated Array class descriptor implementation
@cl4es discovered that Stack Map generation in ClassFile API uses componentType and arrayType for aaload aastore instructions, which are currently quite slow. We can split out array class descriptors from class or interfaces to support faster arrayType and componentType operations.
Tentative, as I currently have no way to measure the actual impact of this patch on the startup performance; however, this made the ClassDesc implementations much cleaner.
Progress
- [x] Change must be properly reviewed (1 review required, with at least 1 Reviewer)
- [x] Change must not contain extraneous whitespace
- [x] Commit message must refer to an issue
- [x] Change requires CSR request JDK-8340963 to be approved
Issues
- JDK-8338544: Dedicated Array class descriptor implementation (Sub-task - P4)
- JDK-8340963: Make some ClassDesc methods no longer default (CSR)
Reviewers
- Claes Redestad (@cl4es - Reviewer)
- Jorn Vernee (@JornVernee - Reviewer) Review applies to a1d14186
- Mandy Chung (@mlchung - Reviewer)
Reviewing
Using git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/20665/head:pull/20665
$ git checkout pull/20665
Update a local copy of the PR:
$ git checkout pull/20665
$ git pull https://git.openjdk.org/jdk.git pull/20665/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 20665
View PR using the GUI difftool:
$ git pr show -t 20665
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/20665.diff
Webrev
:wave: Welcome back liach! 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.
![bridgekeeper[bot] avatar](https://avatars.githubusercontent.com/u/41768318?v=4 "Published by a pub.dev verified publisher"){kind=small}
@liach 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:
8338544: Dedicated Array class descriptor implementation
Reviewed-by: redestad, mchung, jvernee
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 17 new commits pushed to the master branch:
- 98403b75df0a0737bdf082231f38c5c0019fe4c9: 8342854: [JVMCI] Block secondary thread reporting a JVMCI fatal error
- 9a7a850e2892990cf6755a0ccb19711816ad1b51: 8341939: SigningOptionsTest fails without Xcode with command line developer tools after JDK-8341443
- de92fe375771315452fc5318abfd228fdd31c454: 8233451: (fs) Files.newInputStream() cannot be used with character special files
- 002de860813ff6bac8c6392f8c10d1c30fc5c09c: 8342673: Test serviceability/jvmti/events/NotifyFramePopStressTest/NotifyFramePopStressTest.java failed: waited too long for notify
- a21c558699646d44d071945c82203e2d68a4dcc3: 8342863: Use pattern matching for instanceof in equals methods of wrapper classes
- e64f0798be64d334b3ec2a918687aafc2031a8b7: 8342582: user.region for formatting number no longer works for 21.0.5
- 426da4bbad3a3eac15e8b17026ebad52b7c568ea: 8341975: Unable to set encoding for IO.println, IO.print and IO.readln
- a522d216b5bebbf103e5a823f0bba22cf1508883: 8342858: Make target mac-jdk-bundle fails on chmod command
- afb62f73499c09f4a7bde6f522fcd3ef1278e526: 8342683: Use non-short forward jump when passing stop()
- 964d8d2234595afaf4dfe48ea5cacdbfd3792d03: 8340445: [PPC64] Wrong ConditionRegister used in ppc64.ad: flagsRegCR0 cr1
- ... and 7 more: https://git.openjdk.org/jdk/compare/d6eddcdaf92f2352266ba519608879141997cd63...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.
@liach 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.
I know this requires a CSR; it's just not created due to this still being a draft.
The main purpose of this is still for speeding up stack map generation; the results are not confirmed yet, so this patch is on hold.
For megamorphic call site thing, the regular workload would be like:
if (c.isArray()) doArrayStuff(c);
if (c.isClassOrInterface()) doClassOrInterfaceStuff(c);
I think polymorphism is fine here, as after the check we usually go straight to perform type-specific operations like componentType(); or sometimes a method expects the input ClassDesc to be always primitive/class or interface/array, which won't lead to profile pollutions. This concern might be more valid for descriptorString I suppose.
Webrevs
- 07: Full - Incremental (f6bfa12c)
- 06: Full (bf9ff94c)
- 05: Full - Incremental (4acec988)
- 04: Full (62deed8b)
- 03: Full (e65fa6aa)
- 02: Full - Incremental (f6964d1a)
- 01: Full (a1d14186)
- 00: Full (81c2fa0e)
@JornVernee I have updated the implementation class name and the factories for clarity.
Thanks for the reviews; can anyone review the associated CSR at https://bugs.openjdk.org/browse/JDK-8340963 as well?
@liach this pull request can not be integrated into master due to one or more merge conflicts. To resolve these merge conflicts and update this pull request you can run the following commands in the local repository for your personal fork:
git checkout feature/array-cd
git fetch https://git.openjdk.org/jdk.git master
git merge FETCH_HEAD
# resolve conflicts and follow the instructions given by git merge
git commit -m "Merge master"
git push
Just noticed that the ArrayClassDesc::displayName was buggy - existing tests had no coverage and I have added test coverage for displayName and a few other missed methods.
Going to push as commit 25c2f48d458bfd92423c311a887679ad3e1e4041.
Since your change was applied there have been 19 commits pushed to the master branch:
- 158b93d19a518d2b9d3d185e2d4c4dbff9c82aab: 8335912: Add an operation mode to the jar command when extracting to not overwriting existing files
- 28d23ada6dde007ed60b8538cc159afc62d76db3: 8340177: Malformed system classes loaded by bootloader crash the JVM in product builds
- 98403b75df0a0737bdf082231f38c5c0019fe4c9: 8342854: [JVMCI] Block secondary thread reporting a JVMCI fatal error
- 9a7a850e2892990cf6755a0ccb19711816ad1b51: 8341939: SigningOptionsTest fails without Xcode with command line developer tools after JDK-8341443
- de92fe375771315452fc5318abfd228fdd31c454: 8233451: (fs) Files.newInputStream() cannot be used with character special files
- 002de860813ff6bac8c6392f8c10d1c30fc5c09c: 8342673: Test serviceability/jvmti/events/NotifyFramePopStressTest/NotifyFramePopStressTest.java failed: waited too long for notify
- a21c558699646d44d071945c82203e2d68a4dcc3: 8342863: Use pattern matching for instanceof in equals methods of wrapper classes
- e64f0798be64d334b3ec2a918687aafc2031a8b7: 8342582: user.region for formatting number no longer works for 21.0.5
- 426da4bbad3a3eac15e8b17026ebad52b7c568ea: 8341975: Unable to set encoding for IO.println, IO.print and IO.readln
- a522d216b5bebbf103e5a823f0bba22cf1508883: 8342858: Make target mac-jdk-bundle fails on chmod command
- ... and 9 more: https://git.openjdk.org/jdk/compare/d6eddcdaf92f2352266ba519608879141997cd63...master
Your commit was automatically rebased without conflicts.
@liach Pushed as commit 25c2f48d458bfd92423c311a887679ad3e1e4041.
:bulb: You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.