[foreign-memaccess] RFR 8224993: Add Unsafe support for MemoryAddress (again)

Maurizio Cimadamore maurizio.cimadamore at oracle.com
Fri May 31 09:40:39 UTC 2019


Fixed and pushed - thanks

Maurizio

On 31/05/2019 10:31, Jorn Vernee wrote:
> You have a spurious import in MemoryAddressImpl of ForceInline now ;)
>
> Rest looks good!
>
> Cheers,
> Jorn
>
> Maurizio Cimadamore schreef op 2019-05-30 19:11:
>> New revision:
>>
>> http://cr.openjdk.java.net/~mcimadamore/panama/8224993_followup_v2/
>>
>> I finally got to the bottom of the issue - in part the problem is
>> caused by the fact that MemoryAddessImpl::copy is too big and doesn't
>> get inlined into the caller. But, another problem was that we were
>> doing redundant liveness checks - since the check was called
>> explicitly in MemoryAddressImpl::copy, but also, indirectly, in
>> MemoryAddressImpl::checkAccess.
>>
>> I now eliminated these calls, and moved all liveness check to
>> MemorySegmentImpl::checkRange.
>>
>> I've also added a missing call to checkAlive in 
>> MemorySegmentImpl::resize.
>>
>> With these changes the compiled method is still too big, but even w/o
>> the ForceInline annotation, performances are good (and same as with
>> the annotation).
>>
>> Maurizio
>>
>>
>> On 30/05/2019 17:09, Jorn Vernee wrote:
>>> Looks good to me.
>>>
>>> I wanted to ask about the addition of the APIs to sun.misc.Unsafe 
>>> last time, but got side-tracked by the test build error and then 
>>> forgot :/. I had assumed sun.misc.Unsafe would not be updated going 
>>> forward? What's the policy on this?
>>>
>>> Thanks,
>>> Jorn
>>>
>>> Maurizio Cimadamore schreef op 2019-05-30 16:59:
>>>> Hi,
>>>> small followup to yesterday's push.
>>>>
>>>> As I was benchmarking bulk copy performances, I realized that there's
>>>> a bug in the methods which I added yesterday on sun.misc.Unsafe -
>>>> which delegate to the wrong Unsafe (itself), resulting in SO.
>>>>
>>>> I also cleaned up uses of Unsafe internally - and added a @ForceInline
>>>> on MemoryAddressImpl::copy which basically makes it as fast as a raw
>>>> unsafe call to Unsafe::copyMemory. We're still investigating as to why
>>>> exactly the annotation is needed to get good inlining.
>>>>
>>>> I've also added a shortcircuit in AbstractMemoryScopeImpl::checkAlive,
>>>> which avoids a call to 'isAlive' if the scope is pinned. This was
>>>> causing some profile pollution (although performances were still good
>>>> - probably because of the bi-morphic inline cache).
>>>>
>>>> Webrev:
>>>> http://cr.openjdk.java.net/~mcimadamore/panama/8224993_followup/
>>>>
>>>> Maurizio


More information about the panama-dev mailing list