[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