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

[Jorn Vernee]

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