Re: RFR: 8287860: Revise usage of volatile in j.u.Locale [v2]
On Mon, 6 Jun 2022 17:31:15 GMT, Naoto Sato wrote: >> Сергей Цыпанов has updated the pull request incrementally with one >> additional commit since the last revision: >> >> 8287860: Mark hash fields with @Stable > > src/java.base/share/classes/java/util/Locale.java line 1084: > >> 1082: Properties props = GetPropertyAction.privilegedGetProperties(); >> 1083: >> 1084: Locale defaultLocale = Locale.defaultLocale; > > I'd use a different local variable name so that it won't clash with the field > name. The same holds for other locations (`isoLanguages` and `languageTag`) Done! - PR: https://git.openjdk.java.net/jdk/pull/9041
Re: RFR: 8287860: Revise usage of volatile in j.u.Locale [v2]
On Mon, 6 Jun 2022 14:36:03 GMT, Сергей Цыпанов wrote: >> - cached hash code of `Locale` and `Locale$LanguageRange` shouldn't be >> volatile, even in case of race in the worst case it is recalculated at most >> once per thread >> - `defaultLocale` field is read multiple times in `initDefault()` >> - `isoLanguages` is accessed multiple times in `getISOLanguages()` >> - `languageTag` is read twice in `toLanguageTag()` > > Сергей Цыпанов has updated the pull request incrementally with one additional > commit since the last revision: > > 8287860: Mark hash fields with @Stable src/java.base/share/classes/java/util/Locale.java line 1084: > 1082: Properties props = GetPropertyAction.privilegedGetProperties(); > 1083: > 1084: Locale defaultLocale = Locale.defaultLocale; I'd use a different local variable name so that it won't clash with the field name. The same holds for other locations (`isoLanguages` and `languageTag`) - PR: https://git.openjdk.java.net/jdk/pull/9041
Re: RFR: 8287860: Revise usage of volatile in j.u.Locale [v2]
On Mon, 6 Jun 2022 13:34:27 GMT, liach wrote: >> These fields can only be written once besides the default values, but they >> don't have to be written in the static initializer or constructor. So when a >> non-zero hash code is written, it's cached as if it's a final field. For a >> zero value, it would always undergo extra calculations like before and write >> multiple times, but the writes don't matter as it would only write zero, >> which would not yield false hash code if a previously written 0 value is >> cached. > >> Shouldn't the fields annotated with `@Stable` be `final` as well? > > You might have seen `@Stable final` fields. Those are only meaningful for > arrays, where such caching happens at each array index. For non-arrays, > `@Stable final` is simply equivalent to `final`, and simple `@Stable` acts as > what I said above. Done! - PR: https://git.openjdk.java.net/jdk/pull/9041
Re: RFR: 8287860: Revise usage of volatile in j.u.Locale [v2]
> - cached hash code of `Locale` and `Locale$LanguageRange` shouldn't be > volatile, even in case of race in the worst case it is recalculated at most > once per thread > - `defaultLocale` field is read multiple times in `initDefault()` > - `isoLanguages` is accessed multiple times in `getISOLanguages()` > - `languageTag` is read twice in `toLanguageTag()` Сергей Цыпанов has updated the pull request incrementally with one additional commit since the last revision: 8287860: Mark hash fields with @Stable - Changes: - all: https://git.openjdk.java.net/jdk/pull/9041/files - new: https://git.openjdk.java.net/jdk/pull/9041/files/d64917f1..d4534346 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=9041=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=9041=00-01 Stats: 5 lines in 2 files changed: 2 ins; 0 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/9041.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/9041/head:pull/9041 PR: https://git.openjdk.java.net/jdk/pull/9041
