On Thu, 1 Jun 2023 21:00:33 GMT, Maurizio Cimadamore <[email protected]>
wrote:
>> src/java.base/share/classes/java/lang/foreign/MemoryLayout.java line 470:
>>
>>> 468: * @throws IllegalArgumentException if the layout path contains
>>> one or more <a href=#deref-path-elements>dereference path elements</a>.
>>> 469: * @throws IllegalArgumentException if the layout path contains
>>> one or more path elements that select one or more
>>> 470: * sequence element indices, such as {@link
>>> PathElement#sequenceElement(long)} and {@link
>>> PathElement#sequenceElement(long, long)}).
>>
>> Looks like the first method link should like to
>> `PathElement#sequenceElement()` instead? (I think using a bound
>> `sequenceElement` is fine right?)
>
> This is deliberate (but subtle). Basically, for `select` we just want to
> select a target layout (we don't care which). So the method has a preference
> for open sequence layout paths: there's no access or offset to model here,
> the method just needs to end up into some layout at the end of the path. One
> might argue that these restrictions are unnecessary - but I didn't feel
> strongly enough to drop them.
Ok, thanks for explaining.
>> src/java.base/share/classes/java/lang/foreign/SegmentAllocator.java line 388:
>>
>>> 386: * knows that they have fully processed the contents of the
>>> allocated segment before the subsequent allocation request
>>> 387: * takes place.
>>> 388: * @implNote While a prefix allocator is <em>thread-safe</em>,
>>> concurrent access on the same recycling
>>
>> Is the term "thread-safe" defined any where? Should it be?
>
> I think the term is used pretty much all over the javadoc (not just FFM's) -
> e.g. look for this preamble:
>
> * <a
> href="{@docRoot}/java.base/java/lang/doc-files/ValueBased.html">value-based</a>,
> * immutable and thread-safe
> ```
Right, I guess I'm asking: can/should we do better? ;) It might be nice to
define the term once and for all for the entire JDK, similar to how
'reachability' also has a central definition. (and then add a link from here).
But, arguably, that seems like too much scope creep for this PR, so let's save
it for a potential followup.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/14098#discussion_r1213714496
PR Review Comment: https://git.openjdk.org/jdk/pull/14098#discussion_r1213716230
