On Tue, 2 Dec 2025 02:51:57 GMT, Chen Liang <[email protected]> wrote:
>> 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?
>
> No. The avoidance of cache update simply trims down the generated code by
> throwing away the meaningless cache update.
>
> The access to cache is already safeguarded by `constant ==
> MethodHandleImpl.CONSTANT_YES`. I should have moved `var cache = adaptedMh;`
> into the if block of `constant == CONSTANT_YES`.
I still find it confusing, especially tri-state logic part.
For background, `isCompileConstant` was introduced as part of LF sharing effort
to get rid of Java-level profiling in optimized code. The pattern is was
designed for was:
if (isCompileConstant(...)) {
return ...;
} else {
... // do some extra work (either in interpreter, C1, or
not-fully-optimized version in C2)
}
In this patch, you don't follow that pattern and aadd new state
(`CONSTANT_PENDING`) to distinguish interpreter/C1 from C2. What's the
motivation? Why do you want to avoid cache updates coming from C2-generated
code?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/28585#discussion_r2582647097
