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/java/lang/foreign/MemorySegment.java line 408:
> 406: * }
> 407: *
> 408: * In order to simplify determination of alignment, in the case of
> either native or heap
This can be expressed more directly as follows:
Clients can use the ... method to check if a memory segment supports the
alignment constraint of a memory layout, as follows:
I'd also advise against using a method in the snippet, as that looks like the
method is part of the API. Perhaps something like this:
MemoryLayout layout = ...
MemorySegment segment = ...
boolean isAligned = segment.maxByteAlignment() >= layout.byteAlignment()
src/java.base/share/classes/java/lang/foreign/MemorySegment.java line 412:
> 410: * {@snippet lang=java:
> 411: * boolean isAligned(MemorySegment segment, long offset, MemoryLayout
> layout) {
> 412: * return segment.asSlice(offset).maxByteAlignment() >=
> layout.byteAlignment;
Suggestion:
* return segment.asSlice(offset).maxByteAlignment() >=
layout.byteAlignment();
src/java.base/share/classes/java/lang/foreign/MemorySegment.java line 612:
> 610:
> 611: /**
> 612: * {@return the <em>maximum</em> byte alignment (which is equal to
> or higher than
Here I would just say:
Return the maximum byte alignment associated with this memory segment (link to
alignment section)
If you then want to say that the max alignment is useful for alignment check,
or allocation, that probably goes in a separate para (but I note you have one
already - e.g. to show how to check alignment).
src/java.base/share/classes/java/lang/foreign/MemorySegment.java line 615:
> 613: * the requested byte alignment during native segment allocation)}
> 614: * <p>
> 615: * The returned alignment is always an even power of two and is
> derived from:
Not sure about the "even". Surely 2^3 is a valid max alignment?
src/java.base/share/classes/java/lang/foreign/MemorySegment.java line 618:
> 616: * <ul>
> 617: * <li>Heap:
> 618: * the segment offset and backing array type.</li>
Note that "segment offset" is `segment.address()`. So I don't think we want to
use a new term here. We can also say, more succinctly:
derived from the segment address <link to address()> and the type of the
backing heap storage <link to heapObject> (for heap segments).
src/java.base/share/classes/java/lang/foreign/MemorySegment.java line 622:
> 620: * the {@linkplain MemorySegment#address() address()}
> function.</li>
> 621: * </ul>
> 622: * The {@linkplain MemorySegment#NULL NULL} segment returns a
> maximum byte alignment
I believe this should better be moved in the javadoc for `NULL` ?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/18779#discussion_r1565520043
PR Review Comment: https://git.openjdk.org/jdk/pull/18779#discussion_r1565520387
PR Review Comment: https://git.openjdk.org/jdk/pull/18779#discussion_r1565525848
PR Review Comment: https://git.openjdk.org/jdk/pull/18779#discussion_r1565522838
PR Review Comment: https://git.openjdk.org/jdk/pull/18779#discussion_r1565530922
PR Review Comment: https://git.openjdk.org/jdk/pull/18779#discussion_r1565526441
