Re: RFR: 8291065: Creating a VarHandle for a static field triggers class initialization [v4]
On Fri, 2 Jun 2023 01:19:26 GMT, Chen Liang wrote: > In addition, should we initialize the ref class or the declaring class only? > I don't think we need to initialize the ref class when declaring class > initialization should be sufficient, but that is a change in behavior. Declaring class will be initialized [1]. refc should be equals to the declaring class because the static field to be looked up can only be found in the declaring class. There should be no behavioral change. [1] https://download.java.net/java/early_access/jdk21/docs/api/java.base/java/lang/invoke/MethodHandles.Lookup.html#findStaticVarHandle(java.lang.Class,java.lang.String,java.lang.Class) - PR Comment: https://git.openjdk.org/jdk/pull/13821#issuecomment-1573031355
Re: RFR: 8291065: Creating a VarHandle for a static field triggers class initialization [v4]
On Thu, 1 Jun 2023 21:26:25 GMT, Chen Liang wrote: >> Also fixed the bug with NPE in `IndirectVarHandle::isAccessModeSupported`. >> >> A few implementation-detail methods in VarHandle are now documented with the >> implied constraints to avoid subtle problems in the future. Changed >> `IndirectVarHandle` to call `asDirect` lazily to accomodate the lazy >> VarHandle changes. Also changed VarHandleBaseTest to report the whole >> incorrect type of exception caught than swallow it and leaving only a >> message. >> >> Current problems: >> - [ ] The lazy var handle is quite slow on the first invocation. >>- As seen in the benchmark, users can first call >> `Lookup::ensureInitialized` to create an eager handle. >>- After that, the lazy handle has a performance on par with the regular >> var handle. >> - [ ] The class-loading-based test is not in a unit test >>- The test frameworks don't seem to offer fine-grained control for >> class-loading detection or reliable unloading >> >> >> BenchmarkMode Cnt Score >> Error Units >> VarHandleLazyStaticInvocation.initializedInvocation avgt 30 0.769 ± >> 0.003 ns/op >> VarHandleLazyStaticInvocation.lazyInvocation avgt 30 0.767 ± >> 0.002 ns/op >> >> >> BenchmarkMode Cnt Score >> Error Units >> LazyStaticColdStart.methodHandleCreateEagerss 10 73140.100 ± >> 7735.276 ns/op >> LazyStaticColdStart.methodHandleCreateLazy ss 10 5.000 ± >> 8482.883 ns/op >> LazyStaticColdStart.methodHandleInitializeCallEagerss 10 91350.100 ± >> 9776.343 ns/op >> LazyStaticColdStart.methodHandleInitializeCallLazy ss 10 145540.200 ± >> 72894.865 ns/op >> LazyStaticColdStart.varHandleCreateEager ss 10 47069.900 ± >> 3513.909 ns/op >> LazyStaticColdStart.varHandleCreateLazyss 10 24780.300 ± >> 5144.540 ns/op >> LazyStaticColdStart.varHandleInitializeCallEager ss 10 85479.700 ± >> 10816.983 ns/op >> LazyStaticColdStart.varHandleInitializeCallLazyss 10 413630.100 ± >> 189370.448 ns/op > > Chen Liang has updated the pull request incrementally with one additional > commit since the last revision: > > Fix exact swap > > Co-authored-by: Mandy Chung In addition, should we initialize the ref class or the declaring class only? I don't think we need to initialize the ref class when declaring class initialization should be sufficient, but that is a change in behavior. - PR Comment: https://git.openjdk.org/jdk/pull/13821#issuecomment-1572991154
Re: RFR: 8291065: Creating a VarHandle for a static field triggers class initialization [v4]
On Thu, 1 Jun 2023 21:26:25 GMT, Chen Liang wrote: >> Also fixed the bug with NPE in `IndirectVarHandle::isAccessModeSupported`. >> >> A few implementation-detail methods in VarHandle are now documented with the >> implied constraints to avoid subtle problems in the future. Changed >> `IndirectVarHandle` to call `asDirect` lazily to accomodate the lazy >> VarHandle changes. Also changed VarHandleBaseTest to report the whole >> incorrect type of exception caught than swallow it and leaving only a >> message. >> >> Current problems: >> - [ ] The lazy var handle is quite slow on the first invocation. >>- As seen in the benchmark, users can first call >> `Lookup::ensureInitialized` to create an eager handle. >>- After that, the lazy handle has a performance on par with the regular >> var handle. >> - [ ] The class-loading-based test is not in a unit test >>- The test frameworks don't seem to offer fine-grained control for >> class-loading detection or reliable unloading >> >> >> BenchmarkMode Cnt Score >> Error Units >> VarHandleLazyStaticInvocation.initializedInvocation avgt 30 0.769 ± >> 0.003 ns/op >> VarHandleLazyStaticInvocation.lazyInvocation avgt 30 0.767 ± >> 0.002 ns/op >> >> >> BenchmarkMode Cnt Score >> Error Units >> LazyStaticColdStart.methodHandleCreateEagerss 10 73140.100 ± >> 7735.276 ns/op >> LazyStaticColdStart.methodHandleCreateLazy ss 10 5.000 ± >> 8482.883 ns/op >> LazyStaticColdStart.methodHandleInitializeCallEagerss 10 91350.100 ± >> 9776.343 ns/op >> LazyStaticColdStart.methodHandleInitializeCallLazy ss 10 145540.200 ± >> 72894.865 ns/op >> LazyStaticColdStart.varHandleCreateEager ss 10 47069.900 ± >> 3513.909 ns/op >> LazyStaticColdStart.varHandleCreateLazyss 10 24780.300 ± >> 5144.540 ns/op >> LazyStaticColdStart.varHandleInitializeCallEager ss 10 85479.700 ± >> 10816.983 ns/op >> LazyStaticColdStart.varHandleInitializeCallLazyss 10 413630.100 ± >> 189370.448 ns/op > > Chen Liang has updated the pull request incrementally with one additional > commit since the last revision: > > Fix exact swap > > Co-authored-by: Mandy Chung `staticFieldBase` and `staticFieldOffset` can be called before initialization once a class has been linked. They are unsafe operations and so the code has to ensure class initialization before it accesses a static field. - PR Comment: https://git.openjdk.org/jdk/pull/13821#issuecomment-1572912754
Re: RFR: 8291065: Creating a VarHandle for a static field triggers class initialization [v4]
> Also fixed the bug with NPE in `IndirectVarHandle::isAccessModeSupported`. > > A few implementation-detail methods in VarHandle are now documented with the > implied constraints to avoid subtle problems in the future. Changed > `IndirectVarHandle` to call `asDirect` lazily to accomodate the lazy > VarHandle changes. Also changed VarHandleBaseTest to report the whole > incorrect type of exception caught than swallow it and leaving only a message. > > Current problems: > - [ ] The lazy var handle is quite slow on the first invocation. >- As seen in the benchmark, users can first call > `Lookup::ensureInitialized` to create an eager handle. >- After that, the lazy handle has a performance on par with the regular > var handle. > - [ ] The class-loading-based test is not in a unit test >- The test frameworks don't seem to offer fine-grained control for > class-loading detection or reliable unloading > > > BenchmarkMode Cnt Score > Error Units > VarHandleLazyStaticInvocation.initializedInvocation avgt 30 0.769 ± > 0.003 ns/op > VarHandleLazyStaticInvocation.lazyInvocation avgt 30 0.767 ± > 0.002 ns/op > > > BenchmarkMode Cnt Score > Error Units > LazyStaticColdStart.methodHandleCreateEagerss 10 73140.100 ± > 7735.276 ns/op > LazyStaticColdStart.methodHandleCreateLazy ss 10 5.000 ± > 8482.883 ns/op > LazyStaticColdStart.methodHandleInitializeCallEagerss 10 91350.100 ± > 9776.343 ns/op > LazyStaticColdStart.methodHandleInitializeCallLazy ss 10 145540.200 ± > 72894.865 ns/op > LazyStaticColdStart.varHandleCreateEager ss 10 47069.900 ± > 3513.909 ns/op > LazyStaticColdStart.varHandleCreateLazyss 10 24780.300 ± > 5144.540 ns/op > LazyStaticColdStart.varHandleInitializeCallEager ss 10 85479.700 ± > 10816.983 ns/op > LazyStaticColdStart.varHandleInitializeCallLazyss 10 413630.100 ± > 189370.448 ns/op Chen Liang has updated the pull request incrementally with one additional commit since the last revision: Fix exact swap Co-authored-by: Mandy Chung - Changes: - all: https://git.openjdk.org/jdk/pull/13821/files - new: https://git.openjdk.org/jdk/pull/13821/files/6ed6b805..d347ee7e Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=13821&range=03 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=13821&range=02-03 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/13821.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/13821/head:pull/13821 PR: https://git.openjdk.org/jdk/pull/13821
