New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
8305104: Remove the old core reflection implementation #14371
Conversation
👋 Welcome back mchung! A progress list of the required criteria for merging this PR into |
Webrevs
|
/label remove security |
@mlchung |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hotspot code and test changes look fine. Thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The old + new implementations have been in four releases, which is good for shaking out any regressions/issues. Removing the old implementation in JDK 22 is good.
@mlchung 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:
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 no new commits pushed to the ➡️ To integrate this PR with the above commit message to the |
* 1. during VM early startup and method handle support is fully initialized | ||
* 2. a Java native method | ||
* 3. -Djdk.reflect.useNativeAccessorOnly=true is set | ||
* 4. the member takes a variable number of arguments and the last parameter | ||
* is an array (see details below) | ||
* 5. the member's method type has an arity >= 255 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* 1. during VM early startup and method handle support is fully initialized | |
* 2. a Java native method | |
* 3. -Djdk.reflect.useNativeAccessorOnly=true is set | |
* 4. the member takes a variable number of arguments and the last parameter | |
* is an array (see details below) | |
* 5. the member's method type has an arity >= 255 | |
* 1. during VM early startup before method handle support is fully initialized | |
* 2. a Java native method | |
* 3. -Djdk.reflect.useNativeAccessorOnly=true is set | |
* 4. the member takes a variable number of arguments and the last parameter | |
* is not an array (see details below) | |
* 5. the member's method type has an arity >= 255 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed. Thanks.
/integrate |
Going to push as commit 9bfe415.
Your commit was automatically rebased without conflicts. |
JEP 416 integrated in JDK 18 and since then, only a couple minor issues has been reported. Those issues were related with exception being thrown with invalid arguments. We propose to remove the old core reflection implementation in JDK 22. The
-Djdk.reflect.useDirectMethodHandle=false
workaround to revert to the old implementation will stop to work.Progress
Issues
Reviewers
Reviewing
Using
git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/14371/head:pull/14371
$ git checkout pull/14371
Update a local copy of the PR:
$ git checkout pull/14371
$ git pull https://git.openjdk.org/jdk.git pull/14371/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 14371
View PR using the GUI difftool:
$ git pr show -t 14371
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/14371.diff
Webrev
Link to Webrev Comment