[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