[foreign-memaccess] RFR 8226527: Add package-info javadoc

Maurizio Cimadamore maurizio.cimadamore at oracle.com
Thu Jun 27 17:29:10 UTC 2019


On 27/06/2019 18:16, Jorn Vernee wrote:
> Hi,
>
> Some comments:
>
> - I think the example in the package-info is incorrect. The 
> MemoryAddress needs to be offset by `i * 4` each time, since the 
> offset is in bytes.
yes - good catch
> - The Layout.java javadoc says: "for a sequence layout S whose layout 
> element is E, the size of S is that of E", but isn't it E multiplied 
> by the size of the sequence? (and unknown for bounded sequences?)
yes, again good catch
> - MemorySegment javadoc says that "closing an array memory segment can 
> result in the backing Java array to be garbage collected", but the 
> 'ofArray' factory says: "Moreover, the resulting segment is pinned and 
> cannot therefore be closed.". This seems inconsistent, besides, there 
> doesn't seem to be anything preventing me from calling `close()` on an 
> array segment.
Sorry, you are right in that an array segment should not be pinned. In 
fact, neither the buffer segment, nor the array segment should be pinned 
- you can close them - but that doesn't guarantee that resources will be 
deallocated - which is ok for these segments.
> - In MemoryAccessVarHandles::offsetAddress/scaleAddress javadoc starts 
> with: `Creates a memory access memory access var handle` which should 
> be just `Creates a memory access var handle` I think.

Yep

Thanks for the comments

Maurizio

>
> The rest looks good!
>
> Cheers,
> Jorn
>
> On 2019-06-27 15:37, Maurizio Cimadamore wrote:
>> Hi
>> i've done a comprehensive pass over the javadoc of the memory access
>> API, to add a package-level javadoc, and also to improve the quality
>> of the javadoc pretty much across the board.
>>
>> The patch is here:
>>
>> http://cr.openjdk.java.net/~mcimadamore/panama/8226527/
>>
>> Or, if you prefer, a link to the new API javadoc is here:
>>
>> http://cr.openjdk.java.net/~mcimadamore/panama/memaccess_javadoc_v2
>>
>> I've also did some minor tweaks on some API points, to make them more
>> consistent - to name a few:
>>
>> - renamed GroupLayout.struct/union to ofStruct/ofUnion
>> - renamed the methods in MemoryAccessVarHandle to make it more
>> consistent with naming convention in the combinators defined in
>> MethodHandles
>> - removed references to 'endianness' and replaced with 'order' (this
>> is what ByteBuffer does, I think it's better to stick with existing
>> trends)
>> - removed some of the 'Kind' enums which were not adding a lot of
>> value. They are still there in the impl, but they are no longer
>> exposed. We can always do so e.g. if we decided to add way more value
>> layout kinds.
>>
>> The resulting API feels pleasingly compact and tight (but yet 
>> expressive).
>>
>> Comments welcome.
>>
>> Maurizio


More information about the panama-dev mailing list