On Fri, 11 Feb 2022 08:25:16 GMT, Peter Levart <[email protected]> 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
