On Tue, 6 Dec 2022 09:47:29 GMT, Doug Simon <[email protected]> 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 two additional
> commits since the last revision:
>
> - incorporate review feedback [skip ci]
> - removed hard-coded module name [skip ci]
src/java.base/share/classes/jdk/internal/vm/TranslatedException.java line 44:
> 42: */
> 43: @SuppressWarnings("serial")
> 44: final class TranslatedException extends Exception {
Would it be possible to re-format this to avoid the wildly long (150+) lines?
This seems to be moving jdk.vm.ci.hotspot.TranslatedException and hard to see
what is going on.
src/java.base/share/classes/jdk/internal/vm/VMSupport.java line 66:
> 64: * only contains String keys and values.
> 65: */
> 66: private static byte[] serializePropertiesToByteArray(Properties p,
> boolean filter) throws IOException {
I think we need more context as to why there are non-String system properties
in use here.
src/java.base/share/classes/jdk/internal/vm/VMSupport.java line 106:
> 104: }
> 105: if (props.get("oome") != null) {
> 106: throw new OutOfMemoryError("forced OOME");
I don't think code in java.base should be checking for a property named "oome".
Is this for a test that sets this system property on the command line?
-------------
PR: https://git.openjdk.org/jdk/pull/11513
