On Fri, 23 Jan 2026 18:26:09 GMT, Chen Liang <[email protected]> wrote:
>> Since access descriptor is created for each VH operation site, we can >> optimistically cache the adapted method handle in a site if the site >> operates on a constant VH. Used a C2 IR test to verify such a setup through >> an inexact VarHandle invocation can be constant folded through (previously, >> it was blocked by `asType`) > > Chen Liang has updated the pull request with a new target base due to a merge > or a rebase. The incremental webrev excludes the unrelated changes brought in > by the merge/rebase. The pull request contains 23 additional commits since > the last revision: > > - Design detail updates, thanks to jorn > - Merge branch 'master' of https://github.com/openjdk/jdk into > fix/vh-adapt-cache > - Missed IR test review, rearrange benches > - Merge branch 'master' of https://github.com/openjdk/jdk into > fix/vh-adapt-cache > - Stage > - Merge branch 'master' of https://github.com/openjdk/jdk into > fix/vh-adapt-cache > - Review > - Merge branch 'master' of https://github.com/openjdk/jdk into > fix/vh-adapt-cache > - Bugs and verify loader leak > - Try to avoid loader leak > - ... and 13 more: https://git.openjdk.org/jdk/compare/d8405cdd...77ea5565 src/java.base/share/classes/java/lang/invoke/VarHandle.java line 2043: > 2041: // > 2042: // Condition 1 and 2 indicates this access descriptor may see a > VarHandle > 2043: // different from the captured VarHandle. Condition 3 requires > the Suggestion: // Condition 1 and 2 indicates this access descriptor may see a VarHandle // different from the captured VarHandle. I don't see how this follows from condition 1 and 2 holding. A failure in either condition means we are may see multiple VH instance. I suggest: Suggestion: // If either condition 1 or 2 doesn't hold, we may see different var handle instances using the // same shared AccessDescriptor. In those cases we only cache the adaptation for one of the // var handle instances (the first one). Other instances will always use the slow path. src/java.base/share/classes/java/lang/invoke/VarHandle.java line 2049: > 2047: // such as compareAndSet can appear at two sites, where each > site > 2048: // has its own constant VarHandle. Such a usage pattern hurts > adaption, > 2049: // but is perfectly dealt by the getMethodType_V constant > folding branch. I think this information should be put on the template string instead, since it's mostly referencing that code. I think it's enough to say that we 'skip potentially costly adaptation' by going through the `getMethodType_V` branch. >From the text as written, it's not clear why such usage patterns hurt >adaptation. I think It would be more accurate to say that: "in such cases, >only one of the two var handles will have its adaptation cached". src/java.base/share/classes/java/lang/invoke/VarHandle.java line 2055: > 2053: // invocation type of the underlying MemberName, or MH for > indirect VH), > 2054: // perform a foldable lookup with a hash table, and hope C2 > inline it > 2055: // all. Such an optimization applies for general > MethodHandle.asType. Suggestion: // all. Such an optimization applies to general MethodHandle.asType. src/java.base/share/classes/java/lang/invoke/VarHandle.java line 2055: > 2053: // invocation type of the underlying MemberName, or MH for > indirect VH), > 2054: // perform a foldable lookup with a hash table, and hope C2 > inline it > 2055: // all. Such an optimization applies for general > MethodHandle.asType. This reads as "put a ... to another ... perform", which doesn't sounds grammatically correct. Maybe: Suggestion: // In the long run, we wish to cache each specific-type invoker that converts // from one fixed type (symbolicMethodTypeInvoker) to another (the // invocation type of the underlying MemberName, or MH for indirect VH), // using a foldable lookup with a hash table, and hope C2 inline it // all. Such an optimization applies for general MethodHandle.asType. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/28585#discussion_r2722389574 PR Review Comment: https://git.openjdk.org/jdk/pull/28585#discussion_r2722416361 PR Review Comment: https://git.openjdk.org/jdk/pull/28585#discussion_r2722376203 PR Review Comment: https://git.openjdk.org/jdk/pull/28585#discussion_r2722514070
