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