RFR: 8261407: ReflectionFactory.checkInitted() is not thread-safe [v6]

[liach]

On Fri, 11 Feb 2022 08:25:16 GMT, Peter Levart <plevart at openjdk.org> wrote:

>> src/java.base/share/classes/jdk/internal/reflect/ReflectionFactory.java line 685:
>> 
>>> 683:                 instance = c = load();
>>> 684:             }
>>> 685:             return c;
>> 
>> If you do that the "old" way, you loose the ability for JIT to constant-fold the `instance` and by transitivity the Config instance fields, since the check for `VM.isModuleSystemInited()` can't be elided. As suggested, the fast-path check should be done 1st, like:
>> 
>> 
>>     private static @Stable Config instance;
>> 
>>     private static Config instance() {
>>         Config c = instance;
>>         if (c != null) {
>>             return c;
>>         }
>>         
>>         // Defer initialization until module system is initialized so as
>>         // to avoid inflation and spinning bytecode in unnamed modules
>>         // during early startup.
>>         if (!VM.isModuleSystemInited()) {
>>             return DEFAULT;
>>         }
>> 
>>         instance = c = load();
>>         return c;
>>     }
>
> ...having suggested that rearrangement, perhaps the right way to do it is to enable some VM.isXXX queries themselves to be constant-foldable so that other callers too may benefit. Like this:
> https://github.com/plevart/jdk/commit/e918ccc52bbc288f6721af5fa66d8f7a8cc880cf
> WDYT?

I believe your patch to fold these methods is a good choice: for example, `FileSystems.getDefault()` will be constant-foldable as a result.
For shutdown, the benefit may look negligible, but a consistency in style is beneficial.
To make this more efficient, I recommend looking at the callers to `VM.initLevel()` and replace with such boolean checks if possible: for example, `ClassLoader.getSystemClassLoader` may be constant-foldable if its default branch of switch on init level become a dedicated fast path.

Since this change affects multiple components and beyond the reflection factory itself, I don't think I will include it here; I will just use the right arrangement.

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

PR: https://git.openjdk.java.net/jdk/pull/6889