Java value layout constants

[Ty Young]

Sorry for the late reply, busy with thanksgiving.


This seems to be exactly what I personally wanted. Looking at the exact 
code, I feel like most if not all of the if-else and other checks in 
places like MemoryAddress getAtIndex could be eliminated if the API was 
arranged differently. Specifically, if ValueLayout was an interface with 
a non-exported, internal implementation, maybe MethodHandles could then 
be used to point to the correct branches directly by a one-time switch 
when the bit alignment is modified. The MethodHandles could then be 
retrieved via the non-exported, internal implementation via a type cast. 
Maybe that makes the performance worse, I haven't tried it. I just 
figured it would look cleaner.


Again, it seems to be what I wanted so +1 from me for what it's worth.


On 11/25/21 9:52 AM, Maurizio Cimadamore wrote:
> Hi,
> This is a followup of the disccussion that started in [1]. In the new 
> changes slated for Java 18, the set of Java value layout constants are 
> all byte-aligned (e.g. alignment constraints are not set). The 
> motivation for this is mostly historical (but there's also a 
> performance twist, see below): the dereference primitives in 
> MemoryAccess used to setup var handles based on non-aligned layouts. 
> So, to preserve compatibility with what we had before, we opted to 
> "relax" alignment constraints on the JAVA_XYZ layout constants in 
> ValueLayout. During the development of the new dereference API, some 
> issues arised around alignment checks and memory copy [2]; which also 
> contributed to consolidate the feeling that Java layout constants 
> should be unaligned.
>
> Now, while it's always possible, for clients, to go back to the 
> desired alignment constraints (e.g. by defining custom layout 
> constants), from the discussion it emerged that it can be somewhat 
> confusing/surprising having a layout constant called JAVA_INT, whose 
> alignment is not the VM alignment for a Java int value.
>
> For this reason, I'd like to propose a small tweak, which would 
> essentially revert alignment constraints for Java layout constants to 
> what they were in 17. In other words, let's keep the "good" JAVA_XYZ 
> names for the _true_ Java layouts (including alignment as seen by VM). 
> If clients want to create unaligned constants they can do so, as they 
> can also create big-endian constants where needed. In the majority of 
> cases, since access will be aligned (for performance reasons), this 
> will not really change much for clients. But some of those clients 
> that need to pack data structures more (Lucene?) will need to define 
> their own packed/unaligned layout constants.
>
> Does that seem like an acceptable compromise?
>
> A patch for these changes is available here:
>
> https://github.com/mcimadamore/jdk/tree/value_layout_align
>
> While testing it, I was reminded (once more) that access with 
> alignment constraints is currently slower than access w/o alignment 
> constraints - which has to do with C2 not hoisting alignment checks in 
> cases like this:
>
> ((segmentBaseAddress + accessedOffset) & alignmentMask) == 0
>
> Here, segmentBaseAddress is a loop invariant, and the accessedOffset 
> depends on the loop variable. So, it is in principle possible for the 
> VM to hoist the check for baseAddress and to eliminate the alignment 
> check for the offset (which would come from BCE analysis). But this is 
> not how things work today. The patch works around this, by using 
> different var handles for when the accessed offset is provably aligned 
> (e.g. when using the getAtIndex/setAtIndex APIs). Even with those 
> workarounds, calling getAtIndex/setAtIndex on a MemoryAddress is still 
> slower than on a MemorySegment, because of the way in which we try to 
> workaround the long loop optimization problem. Luckily a fix for that 
> problem [3] has been integrated in JDK 18, which means we will remove 
> these implementation workaround, which will help making performance 
> more stable across the board.
>
> If the changes in this patch seem good, I'm happy to try and integrate 
> this into 18.
>
> Cheers
> Maurizio
>
> [1] - 
> https://mail.openjdk.java.net/pipermail/panama-dev/2021-November/015805.html
> [2] - 
> https://github.com/openjdk/panama-foreign/pull/555#issuecomment-865115787
> [3] - https://github.com/openjdk/jdk/pull/2045
>
>
>