Java value layout constants

[Uwe Schindler]

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.

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.

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?

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://lucene.apache.org/
https://solr.apache.org/

> -----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
> >