On Mon, 24 Feb 2025 11:34:15 GMT, Maurizio Cimadamore <mcimadam...@openjdk.org>
wrote:
>> Chen Liang has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Review remarks, dates, some more simplifications
>
> src/java.base/share/classes/jdk/internal/foreign/Utils.java line 130:
>
>> 128: */
>> 129: public static VarHandle makeRawSegmentViewVarHandle(MemoryLayout
>> enclosing, ValueLayout layout, boolean noStride, long offset) {
>> 130: if (enclosing instanceof ValueLayout direct) {
>
> For now the caching is ok. Moving forrward, I wonder if we shouldn't add more
> reuse between field var handles in cases like these:
>
>
> struct Point {
> int x; int y;
> }
>
>
>
> struct Tuple {
> int u; int v;
> }
>
>
> E.g. if I access the first field in these two structs, then the var handle
> will be identical? In a way, we use the enclosing layout for two reasons:
>
> * to determine the size `S` of the accessed memory region
> * to determine the alignment `A` of the accessed memory region
>
> So, if the static `offset`, accessed `layout`, `S` and `A` are the same for
> two accesses, we should probably reuse the same var handle instance? E.g.
> whether we access `x` in a `Point`, or `u` in a `Tuple` makes little
> difference.
I think direct var handles are quite cheap already and don't really need
caching. What really worth caching should be the combinators and the VH derived
from those combinators.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/23720#discussion_r1968262515
