RFR: 8298095: Refine implSpec for SegmentAllocator [v3]

Maurizio Cimadamore mcimadamore at openjdk.org
Thu Jul 27 00:44:52 UTC 2023


On Wed, 26 Jul 2023 14:44:01 GMT, Per Minborg <pminborg at openjdk.org> wrote:

>> This PR suggests refining the `@implSpec` for the SegmentAllocator::allocate methods as well as clarifying the docs a bit more. Also, a local variable is renamed.
>
> Per Minborg has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Update after comments

src/java.base/share/classes/java/lang/foreign/SegmentAllocator.java line 110:

> 108:      * {@snippet lang=java :
> 109:      *  Objects.requireNonNull(layout);
> 110:      *  VarHandle handle = layout.varHandle();

We can make this simpler by using MemorySegment::set ?

src/java.base/share/classes/java/lang/foreign/SegmentAllocator.java line 303:

> 301:      * specified by the provided {@code layout} (i.e. byte ordering, alignment and size)}
> 302:      *
> 303:      * @implSpec The default implementation of this method first calls {@code this.allocateArray(layout, array.length)}

This should probably use code too. I'd suggest to use allocate + MemorySegment::copy

src/java.base/share/classes/java/lang/foreign/SegmentAllocator.java line 472:

> 470:      * @param byteAlignment the alignment (in bytes) of the block of memory to be allocated.
> 471:      * @throws IllegalArgumentException if {@code byteSize < 0}, {@code byteAlignment <= 0},
> 472:      *                                  or if {@code byteAlignment} is not a power of 2.

I'd leave this out. Not because I disagree stylistically, but simply because it's a style that we don't adhere with in this package. If we want to change that, it's ok, but perhaps better to deal with it in another PR.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/14997#discussion_r1275610750
PR Review Comment: https://git.openjdk.org/jdk/pull/14997#discussion_r1275611327
PR Review Comment: https://git.openjdk.org/jdk/pull/14997#discussion_r1275611839


More information about the core-libs-dev mailing list