Re: RFR: 8284579: Improve VarHandle checks for interpreter [v3]
On Mon, 11 Apr 2022 10:13:40 GMT, Claes Redestad wrote: >> A few additional enhancements aiming to improve VH performance in the >> interpreter: >> >> - Flatten `TypeAndInvokers`: adds a pointer to `VarHandle` (a small increase >> 40->48) but removes an object and an indirection on any instance actually >> used - and might avoid allocating the `MethodHandle[]` unnecessarily on some >> instances >> - Have `checkExactAccessMode` return the directness of the `VarHandle` so >> that we can avoid some `isDirect` method calls. >> >> Baseline, `-Xint` >> >> Benchmark Mode CntScore Error Units >> VarHandleExact.exact_exactInvocation avgt 30 478.324 ? 5.762 ns/op >> VarHandleExact.generic_exactInvocationavgt 30 392.114 ? 1.644 ns/op >> VarHandleExact.generic_genericInvocation avgt 30 822.484 ? 1.865 ns/op >> >> >> Patched, `-Xint` >> >> Benchmark Mode CntScore Error Units >> VarHandleExact.exact_exactInvocation avgt 30 437.704 ? 5.320 ns/op >> VarHandleExact.generic_exactInvocationavgt 30 374.512 ? 3.154 ns/op >> VarHandleExact.generic_genericInvocation avgt 30 757.054 ? 1.237 ns/op >> >> >> No significant performance difference in normal mode. > > Claes Redestad has updated the pull request incrementally with one additional > commit since the last revision: > > checkExactAccessMode -> checkAccessModeThenIsDirect, privatize throw > method, cleanup code generator Dropping `Exact` from `checkExactAccessMode` is good with me and it'd help future readers if you can add a javadoc what this method does. - PR: https://git.openjdk.java.net/jdk/pull/8160
Re: RFR: 8284579: Improve VarHandle checks for interpreter [v3]
On Tue, 12 Apr 2022 01:15:33 GMT, David Holmes wrote: > > checkExactAccessMode -> checkAccessModeThenIsDirect > > Don't you still want "Exact" in there? That "access" check seems odd anyway > as it only checks for one form of mismatch - should it not also check for > `!exact && accessModeType(ad.type) == ad.symbolicMethodTypeExact`? What's checked is that the access mode is nominally OK. It just so happens that the only rule validated up front is that iff the VH is exact then the types must be an exact match. For non-exact VH sufficient checks will be done later, eg. by the `asType` call (if the type of the VH isn't adaptable to `ad.symbolicMethodTypeInvoker` this will throw an exception). With that in mind then the name `checkExactAccessMode` even appears misleading as it can be interpreted that the VH _must_ be `exact`. Which is not the case. Dropping the `Exact` part makes it less ambiguous. - PR: https://git.openjdk.java.net/jdk/pull/8160
Re: RFR: 8284579: Improve VarHandle checks for interpreter [v3]
On Mon, 11 Apr 2022 10:13:40 GMT, Claes Redestad wrote: > checkExactAccessMode -> checkAccessModeThenIsDirect Don't you still want "Exact" in there? That "access" check seems odd anyway as it only checks for one form of mismatch - should it not also check for `!exact && accessModeType(ad.type) == ad.symbolicMethodTypeExact`? - PR: https://git.openjdk.java.net/jdk/pull/8160
Re: RFR: 8284579: Improve VarHandle checks for interpreter [v3]
On Mon, 11 Apr 2022 10:13:40 GMT, Claes Redestad wrote: >> A few additional enhancements aiming to improve VH performance in the >> interpreter: >> >> - Flatten `TypeAndInvokers`: adds a pointer to `VarHandle` (a small increase >> 40->48) but removes an object and an indirection on any instance actually >> used - and might avoid allocating the `MethodHandle[]` unnecessarily on some >> instances >> - Have `checkExactAccessMode` return the directness of the `VarHandle` so >> that we can avoid some `isDirect` method calls. >> >> Baseline, `-Xint` >> >> Benchmark Mode CntScore Error Units >> VarHandleExact.exact_exactInvocation avgt 30 478.324 ? 5.762 ns/op >> VarHandleExact.generic_exactInvocationavgt 30 392.114 ? 1.644 ns/op >> VarHandleExact.generic_genericInvocation avgt 30 822.484 ? 1.865 ns/op >> >> >> Patched, `-Xint` >> >> Benchmark Mode CntScore Error Units >> VarHandleExact.exact_exactInvocation avgt 30 437.704 ? 5.320 ns/op >> VarHandleExact.generic_exactInvocationavgt 30 374.512 ? 3.154 ns/op >> VarHandleExact.generic_genericInvocation avgt 30 757.054 ? 1.237 ns/op >> >> >> No significant performance difference in normal mode. > > Claes Redestad has updated the pull request incrementally with one additional > commit since the last revision: > > checkExactAccessMode -> checkAccessModeThenIsDirect, privatize throw > method, cleanup code generator Marked as reviewed by mchung (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8160
Re: RFR: 8284579: Improve VarHandle checks for interpreter [v3]
> A few additional enhancements aiming to improve VH performance in the > interpreter: > > - Flatten `TypeAndInvokers`: adds a pointer to `VarHandle` (a small increase > 40->48) but removes an object and an indirection on any instance actually > used - and might avoid allocating the `MethodHandle[]` unnecessarily on some > instances > - Have `checkExactAccessMode` return the directness of the `VarHandle` so > that we can avoid some `isDirect` method calls. > > Baseline, `-Xint` > > Benchmark Mode CntScore Error Units > VarHandleExact.exact_exactInvocation avgt 30 478.324 ? 5.762 ns/op > VarHandleExact.generic_exactInvocationavgt 30 392.114 ? 1.644 ns/op > VarHandleExact.generic_genericInvocation avgt 30 822.484 ? 1.865 ns/op > > > Patched, `-Xint` > > Benchmark Mode CntScore Error Units > VarHandleExact.exact_exactInvocation avgt 30 437.704 ? 5.320 ns/op > VarHandleExact.generic_exactInvocationavgt 30 374.512 ? 3.154 ns/op > VarHandleExact.generic_genericInvocation avgt 30 757.054 ? 1.237 ns/op > > > No significant performance difference in normal mode. Claes Redestad has updated the pull request incrementally with one additional commit since the last revision: checkExactAccessMode -> checkAccessModeThenIsDirect, privatize throw method, cleanup code generator - Changes: - all: https://git.openjdk.java.net/jdk/pull/8160/files - new: https://git.openjdk.java.net/jdk/pull/8160/files/68f414a1..2a4fbd6d Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8160=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8160=01-02 Stats: 90 lines in 4 files changed: 0 ins; 5 del; 85 mod Patch: https://git.openjdk.java.net/jdk/pull/8160.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8160/head:pull/8160 PR: https://git.openjdk.java.net/jdk/pull/8160
