Skip to content
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

8309622: Re-examine the cache mechanism in BaseLocale #1349

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

GoeLin
Copy link
Member

@GoeLin GoeLin commented Jan 21, 2025

I am thinking about backporting 8309622: Re-examine the cache mechanism in BaseLocale

We see tests
gc/shenandoah/TestAllocIntArrays.java#aggressive and
gc/shenandoah/TestAllocIntArrays.java#iu-aggressive
fail regularly in our CI which is fixed by this change.

Unfortunately 8309622 is not a trivial change. It has one follow-up bug fix and three fixes to startup performance.
So I would have to backport 5 changes.

Luckily, these changes apply clean, except for deleting of src/java.base/share/classes/sun/util/locale/LocaleObjectCache.java because "8316557: Make fields final in 'sun.util' package" is not in 21.

Also, the change is live since 23 including all but one of the follow-ups, so it's pretty well tested.

This PR is only experimental and for discussion. I would backport 8309622 and the bugfix 8328261 in one change, and the three performance fixes as follow-ups.

Alternatively, we could ProblemList the two tests in 21 pointing to 8309622.

For 17, where we see the failure as well, I would prefer the problem listing. This change would not apply well in 17, e.g. it requires JDK-8310913 as prerequisite.


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
  • JDK-8309622 needs maintainer approval
  • JDK-8330802 needs maintainer approval
  • JDK-8331932 needs maintainer approval
  • JDK-8328261 needs maintainer approval
  • JDK-8338897 needs maintainer approval

Warning

 ⚠️ Found leading lowercase letter in issue title for 8328261: public lookup fails with IllegalAccessException when used while module system is being initialized

Issues

  • JDK-8309622: Re-examine the cache mechanism in BaseLocale (Bug - P4)
  • JDK-8328261: public lookup fails with IllegalAccessException when used while module system is being initialized (Bug - P3)
  • JDK-8330802: Desugar switch in Locale::createLocale (Enhancement - P4)
  • JDK-8338897: Small startup regression remains after JDK-8309622 and JDK-8331932 (Bug - P4)
  • JDK-8331932: Startup regressions in 23-b13 (Bug - P4)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk21u-dev.git pull/1349/head:pull/1349
$ git checkout pull/1349

Update a local copy of the PR:
$ git checkout pull/1349
$ git pull https://git.openjdk.org/jdk21u-dev.git pull/1349/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 1349

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk21u-dev/pull/1349.diff

@GoeLin
Copy link
Member Author

GoeLin commented Jan 21, 2025

/issue JDK-8309622
/issue JDK-8328261
/issue JDK-8330802
/issue JDK-8331932
/issue JDK-8338897

@bridgekeeper
Copy link

bridgekeeper bot commented Jan 21, 2025

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

openjdk bot commented Jan 21, 2025

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

@openjdk openjdk bot changed the title Goetz backport 8309622 all 8309622: Re-examine the cache mechanism in BaseLocale Jan 21, 2025
@openjdk
Copy link

openjdk bot commented Jan 21, 2025

@GoeLin The primary solved issue for a PR is set through the PR title. Since the current title does not contain an issue reference, it will now be updated.

@openjdk
Copy link

openjdk bot commented Jan 21, 2025

@GoeLin
Adding additional issue to issue list: 8328261: public lookup fails with IllegalAccessException when used while module system is being initialized.

@openjdk
Copy link

openjdk bot commented Jan 21, 2025

@GoeLin
Adding additional issue to issue list: 8330802: Desugar switch in Locale::createLocale.

@GoeLin GoeLin changed the title 8309622: Re-examine the cache mechanism in BaseLocale Backport f615ac4bdf94750390d18aa954d41f72eb4ef4d2 Jan 21, 2025
@openjdk openjdk bot changed the title Backport f615ac4bdf94750390d18aa954d41f72eb4ef4d2 8331932: Startup regressions in 23-b13 Jan 21, 2025
@openjdk
Copy link

openjdk bot commented Jan 21, 2025

@GoeLin The primary solved issue for a PR is set through the PR title. Since the current title does not contain an issue reference, it will now be updated.

@openjdk
Copy link

openjdk bot commented Jan 21, 2025

@GoeLin
Adding additional issue to issue list: 8338897: Small startup regression remains after JDK-8309622 and JDK-8331932.

@GoeLin GoeLin changed the title 8331932: Startup regressions in 23-b13 Backport f615ac4bdf94750390d18aa954d41f72eb4ef4d2 Jan 21, 2025
@openjdk openjdk bot changed the title Backport f615ac4bdf94750390d18aa954d41f72eb4ef4d2 8309622: Re-examine the cache mechanism in BaseLocale Jan 21, 2025
@openjdk
Copy link

openjdk bot commented Jan 21, 2025

This backport pull request has now been updated with issue from the original commit.

@openjdk openjdk bot added the backport label Jan 21, 2025
@GoeLin
Copy link
Member Author

GoeLin commented Jan 21, 2025

/issue JDK-8331932

@openjdk
Copy link

openjdk bot commented Jan 21, 2025

@GoeLin
Adding additional issue to issue list: 8331932: Startup regressions in 23-b13.

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

Successfully merging this pull request may close these issues.

1 participant