RFR: 8308276: Change layout API to work with bytes, not bits

[Paul Sandoz]

On Tue, 16 May 2023 13:53:32 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

> As explained in [1], memory layouts and memory segments describe sizes using different units. Memory layouts use bits, whereas memory segments use bytes. This is historical: memory layouts were designed after the Minimal LDL [2], which allowed layout strings to be used to describe both memory *and* register. In practice that ship has sailed, and the FFM API uses layouts to firmly model the contents of regions of memory. While it would be possible, in principle, to tweak segments to work on bits, changing that would have implications on how easily code that is currently using ByteBuffer can migrate to use MemorySegment.
> 
> For these reasons, this patch fixes the asymmetry so that layouts also model sizes in term of bytes.
> 
> The patch is straightforward, although a bit tedious (as you can imagine), as various uses of bit sizes had to be turned in byte sizes. In practice, the migration had not been too hard, for the following reasons:
> 
> * the `withBitAlignment` and `bitSize` methods are no longer in the API, it is easy to fix any code (mainly tests) using it;
> * most code already uses ready-made constants such as `JAVA_INT` - such code continues to work unchanged;
> * the layout API de facto already required clients to work with bit sizes that are a multiple of 8.
> 
> The only problematic case is the presence of the `MemoryLayout::paddingLayout(long size)` factory. As this factory is changed to deal in bytes instead of bits, all constants passed to this factory will need to be updated. This is not a problem for code using jextract (as jextract will take care of generating padding) but will be an issue for code using the layout API directly.
> 
> [1] - https://mail.openjdk.org/pipermail/panama-dev/2023-May/019059.html

src/java.base/share/classes/jdk/internal/foreign/layout/ValueLayouts.java line 155:

> 153:             assert isValidCarrier(carrier);
> 154:             assert carrier != MemorySegment.class
> 155:                     // MemorySegment byteSize must always equal ADDRESS_SIZE_BITS

Suggestion:

                    // MemorySegment byteSize must always equal ADDRESS_SIZE_BYTES

test/jdk/java/foreign/TestIllegalLink.java line 122:

> 120:             {
> 121:                     FunctionDescriptor.ofVoid(C_INT.withByteAlignment(2)),
> 122:                     "Layout bit alignment must be natural alignment"

Suggestion:

                    "Layout byte alignment must be natural alignment"

And for all the other occurrences.

test/jdk/java/foreign/TestLayoutPaths.java line 49:

> 47: public class TestLayoutPaths {
> 48: 
> 49:     @Test(expectedExceptions = IllegalArgumentException.class)

Remove this test, since its is the same as the next one.

test/jdk/java/foreign/TestLayoutPaths.java line 185:

> 183:         for (int i = 0 ; i < 4 ; i++) {
> 184:             long bitOffset = g.byteOffset(groupSelector.apply(i));
> 185:             assertEquals(offsets[i], bitOffset);

Suggestion:

            long byteOffset = g.byteOffset(groupSelector.apply(i));
            assertEquals(offsets[i], byteOffset);

test/jdk/java/foreign/TestLayoutPaths.java line 236:

> 234:         for (int i = 0 ; i < 4 ; i++) {
> 235:             long bitOffset = g.byteOffset(sequenceElement(i));
> 236:             assertEquals(offsets[i], bitOffset);

Suggestion:

            long byteOffset = g.byteOffset(sequenceElement(i));
            assertEquals(offsets[i], byteOffset);

test/jdk/java/foreign/TestLayoutPaths.java line 246:

> 244:         byteOffsetHandle = byteOffsetHandle.asSpreader(long[].class, indexes.length);
> 245:         long actualBitOffset = (long) byteOffsetHandle.invokeExact(indexes);
> 246:         assertEquals(actualBitOffset, expectedByteOffset);

Suggestion:

        long actualByteOffset = (long) byteOffsetHandle.invokeExact(indexes);
        assertEquals(actualByteOffset, expectedByteOffset);

test/jdk/java/foreign/TestLayouts.java line 286:

> 284: 
> 285:     @Test(dataProvider="layoutsAndAlignments")
> 286:     public void testBadBitAlignment(MemoryLayout layout, long byteAlign) {

Suggestion:

    public void testBadByteAlignment(MemoryLayout layout, long byteAlign) {

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/14013#discussion_r1197051902
PR Review Comment: https://git.openjdk.org/jdk/pull/14013#discussion_r1197057928
PR Review Comment: https://git.openjdk.org/jdk/pull/14013#discussion_r1197053576
PR Review Comment: https://git.openjdk.org/jdk/pull/14013#discussion_r1197054978
PR Review Comment: https://git.openjdk.org/jdk/pull/14013#discussion_r1197055501
PR Review Comment: https://git.openjdk.org/jdk/pull/14013#discussion_r1197056040
PR Review Comment: https://git.openjdk.org/jdk/pull/14013#discussion_r1197056551