On Tue, 2 Dec 2025 02:13:15 GMT, Chen Liang <[email protected]> wrote:
>> 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. So, it seems like what you are trying to achieve is a 1-1 mapping from `AccessDescriptor` to `vh` through `adaptedMh`. So, once `cache != null` you can trust that it corresponds to the `vh` instance passed as a constant. But cache pollution can easily break the invariant, so you try to eliminate the pollution by avoiding cache updates when vh is not constant. Do I get it right? >> 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. I'd say it's a bad sign. Intermittent bugs manifest exactly in such a way. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/28585#discussion_r2579374286 PR Review Comment: https://git.openjdk.org/jdk/pull/28585#discussion_r2579375565
