[foreign] RFR 8209496: Minimize allocation in NativeInvoker::invokeNormal

[Maurizio Cimadamore]

On 15/08/18 00:03, John Rose wrote:
> Good change.
>
> Those on-heap native structures are handy, aren't they?
>
> We need to get those Pointers updated to value instances, to reduce temp. objects from offsetting.
>
> The names of the form "foo_ptr" hurt my eyes.  Surely they should be "fooPtr"?
Will do
>
> Suggest "assert(n == 1)" at points where a 1-array used to be returned.  In general,
> it's hard to tell that the callers and callees are in agreement about the number of
> argument slots being transferred.
Will do
>
> *All* of the occurrences of "/ 8" make me uncomfortable, because I ask myself,
> can the value ever be non-zero mod eight?  I suggest a Util method something
> like this to replace all uses of "/ 8":
>
>    long divExact(long x, long y) { if (x % y != 0)  throw …; return x/y; }
Good point, I'll defer that in a followup changeset, as this probably 
ends up touching quite a bit of code.
>
> Suggestion:  Allow returnStructPtr to be null for struct returns that return via
> registers, and just use the BoundedPointer.fromLongArray directly.  Saves a copy
> and a native allocation.  Requires clarification that by-value struct returns are not
> directly addressable by native code.
It's a good suggestion, I can see the perf benefits. But I'm not 
convinced we can go down this path. From a Java perspective, there's 
nothing for the type system to distinguish a stack-allocated vs. 
Java-heap-allocated struct. Which means that if objects we get back from 
native calls have extra restrictions layered on top, it might become 
painful for clients to wrestle with these limitations.

I think this is something we could do opportunistically if we can prove 
that we never need to pass the value struct back into native code again 
- e.g. a civilization layer could add some metadata to a given native 
function to say that its result is not meant to be used again - in which 
case your optimization would sound great.

Maurizio
>
> — John
>
> On Aug 14, 2018, at 6:12 AM, Maurizio Cimadamore <maurizio.cimadamore at oracle.com> wrote:
>> Hi,
>> this patch avoids some expensive allocations in the NativeInvoker::invokeNormal code path. This code has to perform two tasks:
>>
>> * given an input array of (Java) object arguments, it has to copy the marshalled version of such arguments into a long array
>>
>> * given a long array contaning return value, it has to extract a Java object out of it (or copying the contents in the desired memory location)
>>
>> To carry out most of these operation, it suffices to operate at a low level, by viewing the long arrays managed by the system ABI as a 'heap' pointer (see new routine BoundedPointer::fromLongArray). From there, we can simply get/set values, using the pointer setter/getters which will directly affect the underlying array, with no need to go via a third pointer.
>>
>> Webrev:
>>
>> http://cr.openjdk.java.net/~mcimadamore/panama/8209496/
>>
>> P.S.
>>
>> Given the touched area is quite delicate, some testing on different platforms would be appreciated :-)
>>
>> Cheers
>> Maurizio
>>