On Mon, 15 Apr 2024 07:50:24 GMT, Per Minborg <pminb...@openjdk.org> wrote:
> This PR proposes to add a new method `MemorySegment::maxByteAlignment` that
> returns the maximum byte alignment of a segment (both heap and native
> segments).
>
> Clients can then use this method to determine if a segment is properly
> aligned for any given layout (e.g. following a `MemorySegment::reinterpret`
> operation).
src/java.base/share/classes/jdk/internal/foreign/HeapMemorySegmentImpl.java
line 86:
> 84: alignment = maxAlignMask();
> 85: }
> 86: return Math.min(maxAlignMask(), alignment);
Similarly here:
Suggestion:
return address() == 0
? maxAlignMask()
: Math.min(maxAlignMask(), Long.lowestOneBit(address()));
src/java.base/share/classes/jdk/internal/foreign/NativeMemorySegmentImpl.java
line 68:
> 66: ? 1L << 62
> 67: : alignment;
> 68: }
This relies on the fact that `lowestOneBit` returns `0` only if `address()`
returns `0`, but I suggest making the intent clearer by checking against
`address()` directly:
Suggestion:
return address() == 0
? 1L << 62
: Long.lowestOneBit(address());
(I also suggest introducing a name for the `1L << 62` constant, e.g.
`MAX_NATIVE_ALIGN`)
test/jdk/java/foreign/TestMemoryAlignment.java line 194:
> 192: new Object[]{MemorySegment.ofArray(new
> float[]{1}), Float.BYTES},
> 193: new Object[]{MemorySegment.ofArray(new
> double[]{1}), Double.BYTES},
> 194: new
> Object[]{MemorySegment.ofBuffer(ByteBuffer.allocate(8)), Byte.BYTES}
What about the other (heap) buffer types?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/18779#discussion_r1565638655
PR Review Comment: https://git.openjdk.org/jdk/pull/18779#discussion_r1565637521
PR Review Comment: https://git.openjdk.org/jdk/pull/18779#discussion_r1565647024
