RFR: 8298099: [JVMCI] decouple libgraal from JVMCI module at runtime [v4]

[David Holmes]

On Mon, 5 Dec 2022 17:45:25 GMT, Doug Simon <dnsimon at openjdk.org> wrote:

>> Libgraal is compiled ahead of time and should not need any JVMCI Java code to be dynamically loaded at runtime. Prior to this PR, this is not the case due to:
>> 
>> * Code to copy system properties from the HotSpot heap into the libgraal heap. This is in `jdk.vm.ci.services.Services.initializeSavedProperties(byte[])` and `jdk.vm.ci.services.Services.serializeSavedProperties()`. This code should be moved to `java.base/jdk.internal.vm.VMSupport`.
>> * Code to translate exceptions from the HotSpot heap into the libgraal heap and vice versa. This code should be moved from `jdk.internal.vm.ci//jdk.vm.ci.hotspot.TranslatedException` to `java.base/jdk.internal.vm.VMSupport`.
>> 
>> This PR makes the above changes. As a result, it's possible to build a JDK image that includes (and uses) libgraal but does not include `jdk.internal.vm.ci` or `jdk.internal.vm.compiler`. This both reduces footprint and better isolates the JAVMCI and the Graal compiler as accessing these components from Java code is now impossible.
>
> Doug Simon has updated the pull request incrementally with one additional commit since the last revision:
> 
>   renamed is_module_resolvable to is_module_observable

Sorry Doug not a full review (not familiar enough with code) but a few drive-by nits.

Thanks.

src/hotspot/share/jvmci/jvmci.cpp line 233:

> 231:     Thread* thread = Thread::current_or_null_safe();
> 232:     if (thread != nullptr && thread->is_Java_thread()) {
> 233:       ResourceMark rm;

You can pass `thread` to the rm constructor to save an internal lookup.

src/hotspot/share/jvmci/jvmci.cpp line 234:

> 232:     if (thread != nullptr && thread->is_Java_thread()) {
> 233:       ResourceMark rm;
> 234:       JavaThreadState state = ((JavaThread*) thread)->thread_state();

Please use `JavaThread::cast(thread)`

src/hotspot/share/jvmci/jvmci.cpp line 241:

> 239:         // resolve the j.l.Thread object unless the thread is in
> 240:         // one of the states above.
> 241:         tty->print("JVMCITrace-%d[%s@" INTPTR_FORMAT "]:%*c", level, thread->type_name(), p2i(thread), level, ' ');

I think the current preferred style is to use PTR_FORMAT here.

src/hotspot/share/runtime/arguments.cpp line 1958:

> 1956:         AddProperty, UnwriteableProperty, InternalProperty);
> 1957:     if (ClassLoader::is_module_observable("jdk.internal.vm.ci")) {
> 1958:       if(!create_numbered_module_property("jdk.module.addmods", "jdk.internal.vm.ci", addmods_count++)) {

Nit: space after `if` please

src/java.base/share/classes/jdk/internal/vm/TranslatedException.java line 2:

> 1: /*
> 2:  * Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.

If you just moved the file the original copyright year should remain - this is not new code.

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

Changes requested by dholmes (Reviewer).

PR: https://git.openjdk.org/jdk/pull/11513