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

[Peter Levart]

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

>> liach has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Make config a pojo, move loading code into config class
>
> 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?

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

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