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

Jorn Vernee jbvernee at xs4all.nl
Thu Jun 27 17:50:02 UTC 2019


Changes look good! But, you forgot to correct the `Creates a memory 
access memory access var handle` for 
MemoryAccessVarHandles::scaleAddress as well.

Imho, no need for another round though.

Jorn

On 2019-06-27 19:39, Maurizio Cimadamore wrote:
> New patch:
> 
> http://cr.openjdk.java.net/~mcimadamore/panama/8226527_v2/
> 
> New javadoc:
> 
> http://cr.openjdk.java.net/~mcimadamore/panama/memaccess_javadoc_v3
> 
> Cheers
> Maurizio
> 
> 
> On 27/06/2019 18:29, Maurizio Cimadamore wrote:
>> 
>> 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