On Tue, 2 Dec 2025 02:00:08 GMT, Vladimir Ivanov <[email protected]> wrote:
>> Indeed, I should move the adaptedMh read into `constant == >> MethodHandleImpl.CONSTANT_YES` block. >> >> `constant != MethodHandleImpl.CONSTANT_NO` prevents capturing any further if >> the VH is known non-constant; we keep this branch in constant case in case >> the adapted MH is not ready when we know the VH is constant. > > I still have a hard time reasoning about state transitions of the cache. > > 1) Why do you limit successful cache read (`cache != null`) to constant `vh` > case (`constant == MethodHandleImpl.CONSTANT_YES`)? > > 2) Why do you avoid cache update in non-constant case (`constant != > MethodHandleImpl.CONSTANT_NO`)? What happens if it runs compiled > `adaptedMethodHandle` method? So an `AccessDescriptor` is created for each sigpoly VH site in the source code. Usually it is `VH.operation()`, but it is legal to use a non-constant VarHandle variable and call an operation on that. If `constant == MethodHandleImpl.CONSTANT_NO`, we are sure that we have the non-constant case, so we cannot trust that cached method handle, and there is no point further caching. We can only read that previous MH conversion cache if `constant == MethodHandleImpl.CONSTANT_YES` because this means our cache is always correct. >> No. The old code worked because it implicitly depended on the backdoor path >> present in the now removed `GUARD_METHOD_TEMPLATE_V` in >> `VarHandleGuardMethodGenerator`. If this test is intact, now its IR compiles >> to doing something in adaptedMethodHandle and calling a MethodHandle. Not >> sure why it doesn't inline through that MethodHandle. > > Ok, so you eliminated a fast-path check for void-return case and now JIT > can't fully optimize it anymore. Do I get it right? Since this particular > bytecode shape is exposed through public API, I don't see why user code can't > step on it. JIT can fully optimize it in JMH benchmarks. I don't know why the IR in this test can't optimize it - I couldn't reproduce this CI failure locally on my linux-x64-debug profile, but this modified test passes on CI. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/28585#discussion_r2579352226 PR Review Comment: https://git.openjdk.org/jdk/pull/28585#discussion_r2579353722
