On Thu, 10 Feb 2022 13:36:11 GMT, liach <d...@openjdk.java.net> wrote:
>> liach has updated the pull request with a new target base due to a merge or
>> a rebase. The incremental webrev excludes the unrelated changes brought in
>> by the merge/rebase. The pull request contains seven additional commits
>> since the last revision:
>>
>> - use peter's model of separate data object
>> - Merge branch 'master' into 8261407-reflectionfactory
>> - Include the stable annotation
>> - Merge branch 'master' into 8261407-reflectionfactory
>> - Merge branch '8261407-reflectionfactory'
>> - Just use volatile directly to ensure read order
>> - 8261407: ReflectionFactory.checkInitted() is not thread-safe
>
> Updated per suggestion of peter and mandy. Requesting another round of review.
Hi @liach ,
This looks good. Maybe we could even make these configuration parameters
constant-foldable so they are basically constants if/when JIT compiles the
code. 1st, you would have to mark the static `config` field as `@Stable` and
move the fast-path check above the check for `VM.isModuleSystemInited()` like
this:
private static @Stable Config config;
private static Config config() {
Config c = config;
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 Config.fallback;
}
config = c = new Config(true);
return c;
}
And that's basically it. The instance final fields in Config class are trusted
by VM since they live in trusted package `jdk.internal.reflect` (see:
s[rc/hotspot/share/ci/ciField.cpp:227](https://github.com/openjdk/jdk/blob/master/src/hotspot/share/ci/ciField.cpp#L227),
so JIT should constant-fold their values when it compiles the code given that
VM has probably already initialized the module system by that time and the
`config` field has a non-null reference.
-------------
PR: https://git.openjdk.java.net/jdk/pull/6889
