On Mon, 15 Apr 2024 14:21:23 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).
>
> Per Minborg has updated the pull request with a new target base due to a 
> merge or a rebase. The incremental webrev excludes the unrelated changes 
> brought in by the merge/rebase. The pull request contains ten additional 
> commits since the last revision:
> 
>  - Update after comments
>  - Merge branch 'master' into ms-reinterpret2
>  - Update 
> src/java.base/share/classes/jdk/internal/foreign/HeapMemorySegmentImpl.java
>    
>    Co-authored-by: Jorn Vernee <jornver...@users.noreply.github.com>
>  - Update 
> src/java.base/share/classes/jdk/internal/foreign/NativeMemorySegmentImpl.java
>    
>    Co-authored-by: Jorn Vernee <jornver...@users.noreply.github.com>
>  - Update src/java.base/share/classes/java/lang/foreign/MemorySegment.java
>    
>    Co-authored-by: Maurizio Cimadamore 
> <54672762+mcimadam...@users.noreply.github.com>
>  - Fix imports and copyright
>  - Update maxByteAlignment docs
>  - Improve doc and tests
>  - Add a MS::maxByteAlignment method

src/java.base/share/classes/java/lang/foreign/MemorySegment.java line 100:

> 98:  * expressed as a {@code long} value. The maximum alignment is always a 
> power of two,
> 99:  * derived from the segment address, and the segment type, as explained 
> in more detail
> 100:  * (see <a href="#segment-alignment">below</a>).

Suggestion:

 * <a href="#segment-alignment">below</a>.

src/java.base/share/classes/java/lang/foreign/MemorySegment.java line 610:

> 608:      * The returned alignment is always a power of two and is derived 
> from:
> 609:      * <ul>
> 610:      *     <li>Heap:

Suggestion:

     *     <li>Heap segments:

src/java.base/share/classes/java/lang/foreign/MemorySegment.java line 612:

> 610:      *     <li>Heap:
> 611:             derived from the segment {@linkplain #address() address()}
> 612:             and the type of the <a href="#segment-alignment">backing 
> heap storage</a>.

Shoudn't the link point to `MemorySegment::heapObject` ?

src/java.base/share/classes/java/lang/foreign/MemorySegment.java line 613:

> 611:             derived from the segment {@linkplain #address() address()}
> 612:             and the type of the <a href="#segment-alignment">backing 
> heap storage</a>.
> 613:      *     <li>Native:

Suggestion:

     *     <li>Native segments:

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18779#discussion_r1565907165
PR Review Comment: https://git.openjdk.org/jdk/pull/18779#discussion_r1565909731
PR Review Comment: https://git.openjdk.org/jdk/pull/18779#discussion_r1565909449
PR Review Comment: https://git.openjdk.org/jdk/pull/18779#discussion_r1565910025

Reply via email to