[foreign-memaccess] RFR 8225515: Remove MemoryScope from public API
Maurizio Cimadamore
maurizio.cimadamore at oracle.com
Tue Jun 11 13:24:03 UTC 2019
Thanks for the review
I had prepared another version which addressed some of your comments,
and some other issues too (e.g. performance issues with heap-based
segment allocation due to call to Unsafe which are wrappers around
native calls).
http://cr.openjdk.java.net/~mcimadamore/panama/8225515_v3/
I'll take care of the other comments too
Maurizio
On 11/06/2019 14:04, Jorn Vernee wrote:
> Hi,
>
> Some comments:
>
> - X-VarHandleMemoryAddressView.java.template still has an import to
> MemoryScope, which is causing compilation to fail when doing a clean
> build.
>
> In MemorySegment.java:
> - The 'ofNative' factory methods mention equivalent code in the
> javadoc which uses `.baseAddress()`, but I think that should be
> dropped, since the methods return a MemorySegment, not a MemoryAddress.
>
> In MemorySegmentImpl.java:
> - Bounds check in resize() and isPinned check in close() are missing
> exception messages. For the bounds check I think it would be nice to
> include the numbers used to do the check in the message, which could
> save some time debugging.
>
> Rest looks good!
>
> Cheers,
> Jorn
>
> Maurizio Cimadamore schreef op 2019-06-10 19:04:
>> Here's a new revision:
>>
>> http://cr.openjdk.java.net/~mcimadamore/panama/8225515_v2/
>>
>> which fixes some javadoc issues
>>
>> Maurizio
>>
>> On 10/06/2019 14:15, Maurizio Cimadamore wrote:
>>> Hi,
>>> this is a followup from the discussion in [1]. Essentially, having a
>>> MemoryScope abstraction seems a bit heavy given the use we would
>>> make of it in the low level access API.
>>>
>>> The discussion sparked very useful discussions on the role of
>>> confinement, which cannot be optional (as initially proposed): doing
>>> so could result in rogue threads to access already freed memory,
>>> thus resulting in a VM crash. Following that discussion, confinement
>>> is now enforced in all memory accesses, so that we have a clean and
>>> safe API.
>>>
>>> Of course, the confined model could be too restrictive in certain
>>> cases, I'm working on a writeup to explain the problem, and possible
>>> solutions - stay tuned.
>>>
>>> Here's the webrev:
>>>
>>> http://cr.openjdk.java.net/~mcimadamore/panama/8225515/
>>>
>>> Note: the webrev includes a patch (thanks Vlad!) to special case
>>> Thread constants in the JIT (otherwise we were seeing repeated calls
>>> to Thread.currentThread() in the profile). Performances of this
>>> patch are really good, and only few percents away from basic Unsafe
>>> access. There is, we believe, some more work to be done when it
>>> comes to array-like access - in which some of the range check
>>> eliminations (RCE) implemented by the JIT fail because we're using
>>> long indices rather than int (which are normally used as array
>>> indices in Java). We're working on that too.
>>>
>>> Cheers
>>> Maurizio
>>>
>>> [1] -
>>> https://mail.openjdk.java.net/pipermail/panama-dev/2019-May/005644.html
>>>
More information about the panama-dev
mailing list