<i18n dev> RFR: 8321206: Make Locale related system properties static properties

Naoto Sato naoto at openjdk.org
Wed Dec 6 18:08:35 UTC 2023


On Wed, 6 Dec 2023 02:14:13 GMT, David Holmes <dholmes at openjdk.org> wrote:

> I'm not following the changes to cdsHeapVerifier.cpp. You've marked the new entries as `C` but the definition is:
> 
> ```
> // [C] A non-final static string that is assigned a string literal during class
> // initialization; this string is never changed during -Xshare:dump.
> ```
> 
> and these are final static strings not non-final. ???

I simply followed the fix to https://bugs.openjdk.org/browse/JDK-8295295 where the last time I added a `StaticProperty`, it broke the CDS test. Since I am not familiar in this area, I am happy to have the reasoning corrected. I would appreciate suggestions.

> You also have a large number of test failures with this PR.

Are you referring to GHA tests? I am not sure they are relevant to this fix, as my run for mach5 did not cause any regression test failures.

> Can I also suggest that you change the bug and PR synopsis to say StaticProperty rather than static properties as I found it quite confusing to understand the issue.

Modified.

>> Making them static properties is safer than relying on the class initialization timing
> "class initialization" refers to the static initialization of a class. I assume that is not what you mean here but the creation of the instance of the class? Even StaticProperty still initializes these things during class initialization, so I'm unclear exactly what this is trying to say.

Modified it to specifically saying `Locale` class loading time.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/16986#issuecomment-1843406352


More information about the i18n-dev mailing list