On Wed, 26 Jul 2023 14:44:01 GMT, Per Minborg <pminb...@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
