RFR: 8295044: Implementation of Foreign Function and Memory API (Second Preview) [v22]

[Maurizio Cimadamore]

On Tue, 15 Nov 2022 15:09:02 GMT, Per Minborg <pminborg at openjdk.org> wrote:

>> Maurizio Cimadamore has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Add `since` tag in Module/ModuleLayer preview methods
>
> src/java.base/share/classes/java/lang/foreign/Arena.java line 89:
> 
>> 87: 
>> 88:     /**
>> 89:      * Returns a native memory segment with the given size (in bytes) and alignment constraint (in bytes).
> 
> It is noted that the current documentation does not require a **new** native memory segment to be returned. Would it not be better with:
> 
> Creates a new native memory segment ...
> 
> The new shared segment might share actual backing memory though.

My feeling is that being overly precise over identity might backfire. It is not important whether the segment is a new instance or not. But there is, perhaps, another invariant that is more semantically relevant: e.g. the returned segments (whether new or not, we don't care) should be backed by "disjoint" regions of memory. That is, if the method returns a segment with address `0` and size `100`, calling the method again cannot return a segment whose address is `50` and size is `100`. In principle, the segment allocator interface allows for this (see `SegmentAllocator::prefixAllocator`) - but for an arena, a behavior such as this would be indesirable, IMHO.

> src/java.base/share/classes/java/lang/foreign/Arena.java line 119:
> 
>> 117: 
>> 118:     /**
>> 119:      * {@return the arena scope}
> 
> Add a period ('.') after the closing curly bracket. This is a general comment.

I don't think we did this consistently in other places, I'd prefer to leave as is.

> src/java.base/share/classes/java/lang/foreign/Arena.java line 136:
> 
>> 134: 
>> 135:     /**
>> 136:      * {@return {@code true} if the provided thread can close this arena}
> 
> I think this is equivalent and simpler:
> 
> {@return if the provided thread can close this arena}.
> 
> But I know there are many examples of {@code true} in the JDK.

I'll leave as is - we can deal with this cosmetic javadoc issues at a later point.

> src/java.base/share/classes/java/lang/foreign/GroupLayout.java line 46:
> 
>> 44: 
>> 45:     /**
>> 46:      * Returns the member layouts associated with this group.
> 
> We may use {@return the member layouts associated with this group}.

Same - I'll leave these tweaks for later.

> src/java.base/share/classes/java/lang/foreign/Linker.java line 264:
> 
>> 262: 
>> 263:     /**
>> 264:      * Returns a symbol lookup for symbols in a set of commonly used libraries.
> 
> Use {@return ...}

Same - I'll leave these tweaks for later.

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

PR: https://git.openjdk.org/jdk/pull/10872