8358426: Improve lazy computation in Locale#25646
8358426: Improve lazy computation in Locale#25646justin-curtis-lu wants to merge 2 commits intoopenjdk:masterfrom
Conversation
|
👋 Welcome back jlu! A progress list of the required criteria for merging this PR into |
|
@justin-curtis-lu 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 42 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 |
|
@justin-curtis-lu The following labels will be automatically applied to this pull request:
When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing lists. If you would like to change these labels, use the /label pull request command. |
Webrevs
|
| private static volatile Locale defaultFormatLocale; | ||
|
|
||
| private transient volatile String languageTag; | ||
| private transient final Supplier<String> languageTag = |
There was a problem hiding this comment.
let's use blessed modifiers order
| private transient final Supplier<String> languageTag = | |
| private final transient Supplier<String> languageTag = |
| System.arraycopy(isoCountries, 0, result, 0, isoCountries.length); | ||
| return result; | ||
| String[] countries = ISO_3166_1_ALPHA2.get(); | ||
| return Arrays.copyOf(countries, countries.length); |
There was a problem hiding this comment.
what about return ISO_3166_1_ALPHA2.get().clone();
There was a problem hiding this comment.
clone is more concise, but I think I prefer the explicitness of copyOf here.
| public Set<String> get() { | ||
| return Set.of(LocaleISOData.ISO3166_3); | ||
| } | ||
| }); |
There was a problem hiding this comment.
What about moving these four stable suppliers and getISO2Table to LocaleISOData to shrink size of Locale.java?
There was a problem hiding this comment.
I like this suggestion, we can also move getISO3Code in addition to those you mentioned, and have all ISO resources and methods come from LocaleISOData. @naotoj, would you be in favor of this?
There was a problem hiding this comment.
I agree, that would be more straightforward. Thanks for the suggestion!
… order for languageTag
| public static Set<String> getISOCountries(IsoCountryCode type) { | ||
| Objects.requireNonNull(type); | ||
| return IsoCountryCode.retrieveISOCountryCodes(type); | ||
| return switch (type) { |
There was a problem hiding this comment.
The body of this method could also be moved to a method in LocaleISOData
There was a problem hiding this comment.
I think it's OK as is, don't want to force re-approvals. Currently it is consistent with the other ISO related methods in Locale, which grab the ISO resources from LocaleISOData but handle some logic on their own as well.
|
Thank you for the reviews! |
|
Going to push as commit cd9b1bc.
Your commit was automatically rebased without conflicts. |
|
@justin-curtis-lu Pushed as commit cd9b1bc. 💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored. |
Please review this PR which improves occurrences of lazy computation in
LocaleandBaseLocale.Existing lazy initialization strategies such as CHM, static nested class, and local inner class are replaced with Stable Values.
Lambda usage is intentionally avoided in this change during
Localecreation and in static fields due to potential startup performance degradation as noted by JDK-8331932.Rather than convert
iso3166CodesMapto a Stable Map, each ISO 3166 resource is represented as a SV. Also, I did not think it was necessary to maintain a SV for both the array and set of ISO3166-1 alpha-2 codes.Progress
Issue
Reviewers
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/25646/head:pull/25646$ git checkout pull/25646Update a local copy of the PR:
$ git checkout pull/25646$ git pull https://git.openjdk.org/jdk.git pull/25646/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 25646View PR using the GUI difftool:
$ git pr show -t 25646Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/25646.diff
Using Webrev
Link to Webrev Comment