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

[Maurizio Cimadamore]

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