Re: RFR: 8265079: Implement VarHandle invoker caching [v3]
On Wed, 14 Apr 2021 22:57:59 GMT, Mandy Chung wrote: >> Jorn Vernee has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Review comment: simplify test > > test/jdk/java/lang/invoke/TestVHInvokerCaching.java line 26: > >> 24: /* @test >> 25: * @bug 8265079 >> 26: * @run testng/othervm -Xverify:all -ea -esa TestVHInvokerCaching > > Nit: the makefile to run jtreg set `-ea -esa`. It's not strictly necessary > in `@run`. If you want to run it standalone, you can run with `jtreg -ea > -esa` option. Ok, I will remove them. - PR: https://git.openjdk.java.net/jdk/pull/3439
Re: RFR: 8265079: Implement VarHandle invoker caching [v4]
> This patch implements 2 leftover TODOs for implementing var handle invoker MH > caching (lambda forms for those were already shared/cached). > > This piggybacks on the existing mechanism for method handle invoker caching. > > Testing: Local testing `java/lang/invoke` tests. Tier 1-3 > > Thanks, > Jorn Jorn Vernee has updated the pull request incrementally with one additional commit since the last revision: Remove manual -ea -esa test flags - Changes: - all: https://git.openjdk.java.net/jdk/pull/3439/files - new: https://git.openjdk.java.net/jdk/pull/3439/files/fa5a721f..0c73b875 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=3439=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3439=02-03 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/3439.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3439/head:pull/3439 PR: https://git.openjdk.java.net/jdk/pull/3439
Re: RFR: 8265079: Implement VarHandle invoker caching [v3]
On Wed, 14 Apr 2021 11:38:15 GMT, Jorn Vernee wrote: >> This patch implements 2 leftover TODOs for implementing var handle invoker >> MH caching (lambda forms for those were already shared/cached). >> >> This piggybacks on the existing mechanism for method handle invoker caching. >> >> Testing: Local testing `java/lang/invoke` tests. Tier 1-3 >> >> Thanks, >> Jorn > > Jorn Vernee has updated the pull request incrementally with one additional > commit since the last revision: > > Review comment: simplify test Marked as reviewed by mchung (Reviewer). test/jdk/java/lang/invoke/TestVHInvokerCaching.java line 26: > 24: /* @test > 25: * @bug 8265079 > 26: * @run testng/othervm -Xverify:all -ea -esa TestVHInvokerCaching Nit: the makefile to run jtreg set `-ea -esa`. It's not strictly necessary in `@run`. If you want to run it standalone, you can run with `jtreg -ea -esa` option. - PR: https://git.openjdk.java.net/jdk/pull/3439
Re: RFR: 8265079: Implement VarHandle invoker caching [v3]
On Wed, 14 Apr 2021 11:38:15 GMT, Jorn Vernee wrote: >> This patch implements 2 leftover TODOs for implementing var handle invoker >> MH caching (lambda forms for those were already shared/cached). >> >> This piggybacks on the existing mechanism for method handle invoker caching. >> >> Testing: Local testing `java/lang/invoke` tests. Tier 1-3 >> >> Thanks, >> Jorn > > Jorn Vernee has updated the pull request incrementally with one additional > commit since the last revision: > > Review comment: simplify test Marked as reviewed by psandoz (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3439
Re: RFR: 8265079: Implement VarHandle invoker caching [v3]
On Wed, 14 Apr 2021 11:38:15 GMT, Jorn Vernee wrote: >> This patch implements 2 leftover TODOs for implementing var handle invoker >> MH caching (lambda forms for those were already shared/cached). >> >> This piggybacks on the existing mechanism for method handle invoker caching. >> >> Testing: Local testing `java/lang/invoke` tests. Tier 1-3 >> >> Thanks, >> Jorn > > Jorn Vernee has updated the pull request incrementally with one additional > commit since the last revision: > > Review comment: simplify test Marked as reviewed by redestad (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3439
Re: RFR: 8265079: Implement VarHandle invoker caching [v2]
On Tue, 13 Apr 2021 15:24:13 GMT, Paul Sandoz wrote:
>> Jorn Vernee has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Review comments:
>> - Use boolean instead of index for var handle cache
>
> test/jdk/java/lang/invoke/TestVHInvokerCaching.java line 88:
>
>> 86: MethodHandles.Lookup lookup = lookup();
>> 87:
>> 88: for (Class type : TEST_TYPES) {
>
> A simpler approach would be to iterate over the fields of `Holder` and use
> `unreflectVarHandle`, then you can remove `TEST_TYPES`.
Yeah, that's a good idea. Fixed
-
PR: https://git.openjdk.java.net/jdk/pull/3439
Re: RFR: 8265079: Implement VarHandle invoker caching [v3]
> This patch implements 2 leftover TODOs for implementing var handle invoker MH > caching (lambda forms for those were already shared/cached). > > This piggybacks on the existing mechanism for method handle invoker caching. > > Testing: Local testing `java/lang/invoke` tests. Tier 1-3 > > Thanks, > Jorn Jorn Vernee has updated the pull request incrementally with one additional commit since the last revision: Review comment: simplify test - Changes: - all: https://git.openjdk.java.net/jdk/pull/3439/files - new: https://git.openjdk.java.net/jdk/pull/3439/files/93681f77..fa5a721f Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=3439=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3439=01-02 Stats: 19 lines in 1 file changed: 4 ins; 12 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/3439.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3439/head:pull/3439 PR: https://git.openjdk.java.net/jdk/pull/3439
Re: RFR: 8265079: Implement VarHandle invoker caching [v2]
On Tue, 13 Apr 2021 12:25:20 GMT, Jorn Vernee wrote:
>> This patch implements 2 leftover TODOs for implementing var handle invoker
>> MH caching (lambda forms for those were already shared/cached).
>>
>> This piggybacks on the existing mechanism for method handle invoker caching.
>>
>> Testing: Local testing `java/lang/invoke` tests. Tier 1-3
>>
>> Thanks,
>> Jorn
>
> Jorn Vernee has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Review comments:
> - Use boolean instead of index for var handle cache
test/jdk/java/lang/invoke/TestVHInvokerCaching.java line 88:
> 86: MethodHandles.Lookup lookup = lookup();
> 87:
> 88: for (Class type : TEST_TYPES) {
A simpler approach would be to iterate over the fields of `Holder` and use
`unreflectVarHandle`, then you can remove `TEST_TYPES`.
-
PR: https://git.openjdk.java.net/jdk/pull/3439
Re: RFR: 8265079: Implement VarHandle invoker caching
On Mon, 12 Apr 2021 16:24:37 GMT, Jorn Vernee wrote: > This patch implements 2 leftover TODOs for implementing var handle invoker MH > caching (lambda forms for those were already shared/cached). > > This piggybacks on the existing mechanism for method handle invoker caching. > > Testing: Local testing `java/lang/invoke` tests. Tier 1-3 > > Thanks, > Jorn Thanks for the reviews. Comment addressed. - PR: https://git.openjdk.java.net/jdk/pull/3439
Re: RFR: 8265079: Implement VarHandle invoker caching [v2]
> This patch implements 2 leftover TODOs for implementing var handle invoker MH > caching (lambda forms for those were already shared/cached). > > This piggybacks on the existing mechanism for method handle invoker caching. > > Testing: Local testing `java/lang/invoke` tests. Tier 1-3 > > Thanks, > Jorn Jorn Vernee has updated the pull request incrementally with one additional commit since the last revision: Review comments: - Use boolean instead of index for var handle cache - Changes: - all: https://git.openjdk.java.net/jdk/pull/3439/files - new: https://git.openjdk.java.net/jdk/pull/3439/files/df10dbdd..93681f77 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=3439=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3439=00-01 Stats: 14 lines in 1 file changed: 4 ins; 0 del; 10 mod Patch: https://git.openjdk.java.net/jdk/pull/3439.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3439/head:pull/3439 PR: https://git.openjdk.java.net/jdk/pull/3439
Re: RFR: 8265079: Implement VarHandle invoker caching
On Mon, 12 Apr 2021 16:24:37 GMT, Jorn Vernee wrote:
> This patch implements 2 leftover TODOs for implementing var handle invoker MH
> caching (lambda forms for those were already shared/cached).
>
> This piggybacks on the existing mechanism for method handle invoker caching.
>
> Testing: Local testing `java/lang/invoke` tests. Tier 1-3
>
> Thanks,
> Jorn
Looks good.
src/java.base/share/classes/java/lang/invoke/Invokers.java line 131:
> 129: }
> 130:
> 131: private MethodHandle cachedVHInvoker(int idx, VarHandle.AccessMode
> ak) {
You can turn `int idx` parameter into `boolean isExact` and choose base index
depending on its value.
-
Marked as reviewed by vlivanov (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/3439
Re: RFR: 8265079: Implement VarHandle invoker caching
On Mon, 12 Apr 2021 16:24:37 GMT, Jorn Vernee wrote: > This patch implements 2 leftover TODOs for implementing var handle invoker MH > caching (lambda forms for those were already shared/cached). > > This piggybacks on the existing mechanism for method handle invoker caching. > > Testing: Local testing `java/lang/invoke` tests. Tier 1-3 > > Thanks, > Jorn LGTM `MethodHandles.varHandle(Exact)Invoker` might be somewhat obscure, but caching them like this is straightforward and basically free. - Marked as reviewed by redestad (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/3439
RFR: 8265079: Implement VarHandle invoker caching
This patch implements 2 leftover TODOs for implementing var handle invoker MH caching (lambda forms for those were already shared/cached). This piggybacks on the existing mechanism for method handle invoker caching. Testing: Local testing `java/lang/invoke` tests. Tier 1-3 Thanks, Jorn - Commit messages: - Add VarHandle invoker handle caching Changes: https://git.openjdk.java.net/jdk/pull/3439/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3439=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8265079 Stats: 115 lines in 2 files changed: 110 ins; 0 del; 5 mod Patch: https://git.openjdk.java.net/jdk/pull/3439.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3439/head:pull/3439 PR: https://git.openjdk.java.net/jdk/pull/3439
