Re: RFR: 8310829: guarantee(!HAS_PENDING_EXCEPTION) failed in ExceptionTranslation::doit [v3]
On Mon, 26 Jun 2023 13:17:25 GMT, Doug Simon wrote: >> The VMSupport class is required for translating an exception between the >> HotSpot and libgraal heaps. >> Loading it lazily can result in a loading exception, obscuring the exception >> being translated. >> To avoid this, VMSupport is loaded eagerly along with the other vmClasses. > > Doug Simon has updated the pull request incrementally with one additional > commit since the last revision: > > each exception translation failure should trigger a JVMCI event I've pushed the loading of `VMSupport` down into the 2 cases where it's actually needed and made it lazy again which should address all concerns about extra noise at VM startup. Thanks for the push back on this - I agree that it's the best solution. - PR Comment: https://git.openjdk.org/jdk/pull/14641#issuecomment-1610243648
Re: RFR: 8310829: guarantee(!HAS_PENDING_EXCEPTION) failed in ExceptionTranslation::doit [v3]
On Mon, 26 Jun 2023 13:17:25 GMT, Doug Simon wrote: >> The VMSupport class is required for translating an exception between the >> HotSpot and libgraal heaps. >> Loading it lazily can result in a loading exception, obscuring the exception >> being translated. >> To avoid this, VMSupport is loaded eagerly along with the other vmClasses. > > Doug Simon has updated the pull request incrementally with one additional > commit since the last revision: > > each exception translation failure should trigger a JVMCI event It may be in the noise but noise adds up over time. It just seems to me that the simplest fix here would have been to convert Klass* vmSupport = SystemDictionary::resolve_or_fail(vmSymbols::jdk_internal_vm_VMSupport(), true, THREAD); guarantee(!HAS_PENDING_EXCEPTION, ""); to Klass* vmSupport = SystemDictionary::resolve_or_fail(vmSymbols::jdk_internal_vm_VMSupport(), true, CHECK); and just return on exception. A very isolated change with zero impact on anything else. - PR Comment: https://git.openjdk.org/jdk/pull/14641#issuecomment-1608455121
Re: RFR: 8310829: guarantee(!HAS_PENDING_EXCEPTION) failed in ExceptionTranslation::doit [v3]
On Mon, 26 Jun 2023 21:56:21 GMT, David Holmes wrote: > I would think it is likely to fail with OOME under the same GC stress test > conditions. > > But if this is just a stress test issue then bailing out when the loading > fails would be far simpler than pre-loading the class. You've added to the VM > startup cost just to avoid a stress test failure. Any app running near the GC limit can cause this and we've seen this with libgraal in practice. When the VM is near the GC limit, any compilation can cause an OOME and then when that OOME wants to be translated back to libgraal, it can be the first time VMSupport is used. The time for loading `VMSupport` eagerly is in the noise. For example, `-verbose:class` shows that about 30 classes (including `VMSupport`) are loaded in 1ms: [0.011s][info][class,load] java.lang.reflect.Executable source: shared objects file [0.012s][info][class,load] java.lang.reflect.Method source: shared objects file [0.012s][info][class,load] java.lang.reflect.Constructor source: shared objects file [0.012s][info][class,load] jdk.internal.vm.ContinuationScope source: shared objects file [0.012s][info][class,load] jdk.internal.vm.Continuation source: shared objects file [0.012s][info][class,load] jdk.internal.vm.StackChunk source: shared objects file [0.012s][info][class,load] jdk.internal.vm.VMSupport source: shared objects file [0.012s][info][class,load] jdk.internal.reflect.MagicAccessorImpl source: shared objects file [0.012s][info][class,load] jdk.internal.reflect.MethodAccessor source: shared objects file [0.012s][info][class,load] jdk.internal.reflect.MethodAccessorImpl source: shared objects file [0.012s][info][class,load] jdk.internal.reflect.DelegatingClassLoader source: shared objects file [0.012s][info][class,load] jdk.internal.reflect.ConstantPool source: shared objects file [0.012s][info][class,load] java.lang.annotation.Annotation source: shared objects file [0.012s][info][class,load] jdk.internal.reflect.CallerSensitive source: shared objects file [0.012s][info][class,load] jdk.internal.reflect.ConstructorAccessor source: shared objects file [0.012s][info][class,load] jdk.internal.reflect.ConstructorAccessorImpl source: shared objects file [0.012s][info][class,load] jdk.internal.reflect.DirectConstructorHandleAccessor$NativeAccessor source: shared objects file [0.012s][info][class,load] jdk.internal.reflect.SerializationConstructorAccessorImpl source: shared objects file [0.012s][info][class,load] java.lang.invoke.MethodHandle source: shared objects file [0.012s][info][class,load] java.lang.invoke.DirectMethodHandle source: shared objects file [0.012s][info][class,load] java.lang.invoke.VarHandle source: shared objects file [0.012s][info][class,load] java.lang.invoke.MemberName source: shared objects file [0.012s][info][class,load] java.lang.invoke.ResolvedMethodName source: shared objects file [0.012s][info][class,load] java.lang.invoke.MethodHandleNatives source: shared objects file [0.012s][info][class,load] java.lang.invoke.LambdaForm source: shared objects file [0.012s][info][class,load] java.lang.invoke.TypeDescriptor$OfMethod source: shared objects file [0.012s][info][class,load] java.lang.invoke.MethodType source: shared objects file [0.012s][info][class,load] java.lang.BootstrapMethodError source: shared objects file [0.012s][info][class,load] java.lang.invoke.CallSite source: shared objects file [0.012s][info][class,load] jdk.internal.foreign.abi.NativeEntryPoint source: shared objects file [0.012s][info][class,load] jdk.internal.foreign.abi.ABIDescriptor source: shared objects file [0.012s][info][class,load] jdk.internal.foreign.abi.VMStorage source: shared objects file [0.013s][info][class,load] jdk.internal.foreign.abi.UpcallLinker$CallRegs source: shared objects file - PR Comment: https://git.openjdk.org/jdk/pull/14641#issuecomment-1608388679
Re: RFR: 8310829: guarantee(!HAS_PENDING_EXCEPTION) failed in ExceptionTranslation::doit [v3]
On Mon, 26 Jun 2023 21:47:08 GMT, Doug Simon wrote: > I doubt VMSupport. would ever fail in practice. I would think it is likely to fail with OOME under the same GC stress test conditions. But if this is just a stress test issue then bailing out when the loading fails would be far simpler than pre-loading the class. You've added to the VM startup cost just to avoid a stress test failure. - PR Comment: https://git.openjdk.org/jdk/pull/14641#issuecomment-1608371203
Re: RFR: 8310829: guarantee(!HAS_PENDING_EXCEPTION) failed in ExceptionTranslation::doit [v3]
On Mon, 26 Jun 2023 21:41:25 GMT, David Holmes wrote: > Then why did you not simply handle the exception from the loading of > VMSupport the same way? The only actual case seen of the original way failing is due to OOME in GC stress tests. If loading of VMSupport is done during VM startup, then no stress test code can cause an OOME while loading VMSupport. I doubt `VMSupport.` would ever fail in practice. - PR Comment: https://git.openjdk.org/jdk/pull/14641#issuecomment-1608330027
Re: RFR: 8310829: guarantee(!HAS_PENDING_EXCEPTION) failed in ExceptionTranslation::doit [v3]
On Mon, 26 Jun 2023 13:06:46 GMT, Doug Simon wrote: > That's fine - if VMSupport fails class initialization, then no "rich" > decoding can occur (by definition) and so throwing the clinit error is the > best we can do. Then why did you not simply handle the exception from the loading of VMSupport the same way? - PR Comment: https://git.openjdk.org/jdk/pull/14641#issuecomment-1608306762
Re: RFR: 8310829: guarantee(!HAS_PENDING_EXCEPTION) failed in ExceptionTranslation::doit [v3]
On Mon, 26 Jun 2023 13:17:25 GMT, Doug Simon wrote: >> The VMSupport class is required for translating an exception between the >> HotSpot and libgraal heaps. >> Loading it lazily can result in a loading exception, obscuring the exception >> being translated. >> To avoid this, VMSupport is loaded eagerly along with the other vmClasses. > > Doug Simon has updated the pull request incrementally with one additional > commit since the last revision: > > each exception translation failure should trigger a JVMCI event looks ok to me. - Marked as reviewed by never (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/14641#pullrequestreview-1498967746
Re: RFR: 8310829: guarantee(!HAS_PENDING_EXCEPTION) failed in ExceptionTranslation::doit [v3]
On Mon, 26 Jun 2023 13:17:25 GMT, Doug Simon wrote: >> The VMSupport class is required for translating an exception between the >> HotSpot and libgraal heaps. >> Loading it lazily can result in a loading exception, obscuring the exception >> being translated. >> To avoid this, VMSupport is loaded eagerly along with the other vmClasses. > > Doug Simon has updated the pull request incrementally with one additional > commit since the last revision: > > each exception translation failure should trigger a JVMCI event BTW, I've manually tested this with libgraal. It's not possible to add a jtreg test until libgraal in part of OpenJDK. - PR Comment: https://git.openjdk.org/jdk/pull/14641#issuecomment-1607466267
Re: RFR: 8310829: guarantee(!HAS_PENDING_EXCEPTION) failed in ExceptionTranslation::doit [v3]
> The VMSupport class is required for translating an exception between the > HotSpot and libgraal heaps. > Loading it lazily can result in a loading exception, obscuring the exception > being translated. > To avoid this, VMSupport is loaded eagerly along with the other vmClasses. Doug Simon has updated the pull request incrementally with one additional commit since the last revision: each exception translation failure should trigger a JVMCI event - Changes: - all: https://git.openjdk.org/jdk/pull/14641/files - new: https://git.openjdk.org/jdk/pull/14641/files/614e1e9f..819a24fd Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=14641&range=02 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=14641&range=01-02 Stats: 4 lines in 2 files changed: 3 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/14641.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14641/head:pull/14641 PR: https://git.openjdk.org/jdk/pull/14641
