-
Notifications
You must be signed in to change notification settings - Fork 6.2k
8357728: Avoid caching synthesized names in synthesized parameters #25961
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
Conversation
|
👋 Welcome back liach! A progress list of the required criteria for merging this PR into |
|
@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: 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 60 new commits pushed to the
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 |
Webrevs
|
|
/csr needed |
|
@liach , please file a CSR for the behavioral change here. |
|
@jddarcy has indicated that a compatibility and specification (CSR) request is needed for this pull request. @liach please create a CSR request for issue JDK-8357728 with the correct fix version. This pull request cannot be integrated until the CSR request is approved. |
|
Done. I have described the context in detail and moved the CSR to proposed. |
|
@liach 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 issue a |
|
@liach 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 |
|
@liach This pull request is now open |
JornVernee
left a comment
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.
Looks good.
|
Thanks for the reviews! /integrate |
|
Going to push as commit f0afd89.
Your commit was automatically rebased without conflicts. |
Currently, fake parameters are created with "arg0" etc. strings that are retained for class file methods with no MethodParameters attribute. The original issue report observes many of these strings present in the heap. To address this issue, we can remove these eagerly created fake names, bringing their behavior in line with nameless parameters from MethodParameters attribute, such as those from inner class constructors. A unit test verifies that no name is retained in the Parameter object when the name is fake.
The original report recommends using a pool to store the arg name strings. I did not take that approach as I don't think it is very necessary; if the issue of retained fake names persist, we can update
Parameter::getNameto return fake names from a pool instead.Progress
Issues
Reviewers
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/25961/head:pull/25961$ git checkout pull/25961Update a local copy of the PR:
$ git checkout pull/25961$ git pull https://git.openjdk.org/jdk.git pull/25961/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 25961View PR using the GUI difftool:
$ git pr show -t 25961Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/25961.diff
Using Webrev
Link to Webrev Comment