Java value layout constants

Mon Nov 29 14:46:46 UTC 2021

On 29/11/2021 14:37, Uwe Schindler wrote:
> Hi,
>
> when you set the alignment to 8 (bits) the alignmentBytes gets 1 and the mask therefore is 0 in the resulting VarHandle. What would be interesting for Lucene: Is the alignment check then always removed by Hotspot (also outside loops)? Because "address & 0" always returns 0 for any address and can easily detected for any code path by Hotspot (e.g., outside loops). I am asking this to make sure that setting the alignment explicit to 8 bits will disable the check completely, in any code path to get optimal performance.
This is already how things work - if alignment is set to 8 bits, no 
alignment check occurs - everything is hoisted.
>
> For the understanding about this issue: The alignment was disabled but should be enabled again. Why is this so important? If anybody wants to make sure the alignment is ok, why not opt-in? The only problem is atomic accesses then, but for that they VarHandle has the VM_ALIGN extra check, so doing getVolatile() on unaligned address always fails.

I think the issue is mostly pedagogical: we have constants called: 
JAVA_INT, JAVA_LONG, and so forth - but they don't have the alignment 
the JVM thinks they have, and that can be surprising, coming from an 
official Java SE API.

Plus, if the default is unaligned, there might be odd exceptions when 
accessing unaligned memory locations on platforms which do not support 
such unaligned access. In other words, of the two (aligned vs. 
unaligned), the case for having constants track "official" alignment 
constraints of Java primitive types seems stronger.

>
> One last thing I noticed in your branch: Why this complexity?:
>
>      @Stable
>      private final VarHandle[] handles = new VarHandle[2];
>
>      static final int ALIGNED_POS = 0;
>      static final int UNALIGNED_POS = 1;
>
>      @ForceInline
>      VarHandle accessHandle(boolean aligned) {
>          int pos = aligned ? ALIGNED_POS : UNALIGNED_POS;
>          if (handles[pos] == null) {
>              // this store to stable field is safe, because return value of 'makeMemoryAccessVarHandle' has stable identity
>              handles[pos] = Utils.makeMemoryAccessVarHandle(this, aligned);
>          }
>          return handles[pos];
>      }
>
> Wouldn't it be easier to have 2 varhandle fields: alignedHandle, unalignedHandle in two different @Stable?

We can easily tweak this. But the two var handles are required to 
workaround the performance penalties around JDK-8277850.

Maurizio

>
> I am also not a fan of methods taking a boolean and return only 2 different values. I would have 2 methods with only the caching conditional: accesHandleUnaligned() and accessHandleAligned(). This would make code using this more readable.
>
> Uwe
>
> -----
> Uwe Schindler
> uschindler at apache.org
> ASF Member, Member of PMC and Committer of Apache Lucene and Apache Solr
> Bremen, Germany
> https://urldefense.com/v3/__https://lucene.apache.org/__;!!ACWV5N9M2RV99hQ!a-C8rUhoRuAlURlp8Ljs8B52jMflRmzAumz_Vpil8bdTa6J_pxDh0chcqO8FLlUh4XYAwf4$
> https://urldefense.com/v3/__https://solr.apache.org/__;!!ACWV5N9M2RV99hQ!a-C8rUhoRuAlURlp8Ljs8B52jMflRmzAumz_Vpil8bdTa6J_pxDh0chcqO8FLlUhuv6vUTw$
>
>> -----Original Message-----
>> From: Maurizio Cimadamore <maurizio.cimadamore at oracle.com>
>> Sent: Monday, November 29, 2021 12:10 PM
>> To: Uwe Schindler <uschindler at apache.org>; panama-dev at openjdk.java.net
>> Subject: Re: Java value layout constants
>>
>> Thanks for the feedback.
>>
>> To be clear, the proposed patch should not change the performance
>> charateristics of your application, as long as clients adopts one of the
>> two strategies below:
>>
>> * manually declare a set of unaligned layouts, and use them for
>> dereference (as seems to be the case for Lucene)
>> * always use get/setAtIndex - in which case we can dynamically infer
>> whether the offset is always a multiple of the alignment, and omit the
>> problematic check on the var handle side (we use a similar trick when
>> constructing var handles from layout paths)
>>
>> When 8277850 is fixed, then accessing at index and at offset should have
>> same performance regardless of alignment.
>>
>> Cheers
>> Maurizio
>>
>>
>> On 27/11/2021 10:00, Uwe Schindler wrote:
>>> Hi Maurizio,
>>>
>>>> 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.
>>> That's all fine. In my first JDK 18 branch of Lucene's new MMapDirectory, I did
>> it like that:
>> https://urldefense.com/v3/__https://github.com/uschindler/lucene/blob/ad3a
>> 81e3d348d6aa417aa785bdfe9e7a39c1ee53/lucene/core/src/java/org/apache/l
>> ucene/store/MemorySegmentIndexInput.java*L36-
>> L40__;Iw!!ACWV5N9M2RV99hQ!fLIcQ7APXm6JbsHKLk9PFBi_YpF84eBNehTaed
>> MyHoBVTKVJjYlFItYeVI2Y6-sg9wyaeVo$
>>> Basically we have layout constants anyways (as the file format has a defined
>> byte order). At the time when I wrote this (in September) I was already
>> thinking: "maybe add withBitAlignment(8) everywhere?"
>>> In general, when you define your own constant for on-disk layouts it is always
>> adviseable to be specifc with byte order and alignment.
>>> Anyway, we are working on Lucene to have alignment in our files at least for
>> those non-packed formats. E.g. we now align file slices (we call them CFS files)
>> always with 8 bytes to not add additional misalignment there. At some point in
>> future we may to enable the alignment checks, but this will be after several
>> years (file format compatibility).
>>> Uwe
>>>