[foreign] RFR 8215289: Cleanup code post SystemABI integration

[Jorn Vernee]

Hi,

I was seeing failures in FunctionAccessTest [1] as well as 
ArraysInFunctionTest [2]. (was just about to send an email about it :)) 
Both are probably related to the recycling of MethodHandles that you 
mention. Maybe you could try adding a FASTPATH=none run to 
ArraysInFunctionTest as well?

As an additional cleanup suggestion; The original patch added 
CallingConvention to SystemABI, but it's currently not being used it 
seems. Maybe this can be removed for now and revisited later?

Jorn

[1] : 
http://cr.openjdk.java.net/~jvernee/panama/test-results/ArraysInFunctionTest.jtr
[2] : 
http://cr.openjdk.java.net/~jvernee/panama/test-results/FunctionAccessTest.jtr

Maurizio Cimadamore schreef op 2018-12-12 17:18:
> Hi,
> this is a followup to the SystemABI work; the goal is to clean up some
> of the code, as I found some minor issues.
> 
> The most important issue is that there is a bug in the shortcircuiting
> logic - e.g. when doing a downcall on an upcall stub. In these cases,
> we return the method handle cached in the upcall handler, but this
> method handle doesn't always have the same signature as the one that
> has been requested by the client. The most striking case is
> UniversalUpcallHandler, whose method handle is always of type
> (Object[]).
> 
> Since we also uncovered another issue with this logic during the
> review process (there are questions as to whether the MH can be safely
> recycled if the new native method type is different from the old one),
> I decided to simplify the code and remove the logic altogether. If
> performances indicate that this is an issue, we can work out a story;
> I did some JMH benchamarks and found no evidence that this is the case
> right now (but we can revise if needed).
> 
> I also did a bunch of other changes:
> 
> * consolidated NativeMethodType; the class is now just an aggregate of
> layout types; the constructor that takes a Method and a Function has
> been moved into Utils, after all this is a binder-specific
> functionality (as the binder happens to know how to go from an
> interface/function to a NMT). I also added accessors in the spirit of
> j.l.i.MethodType to get return and parameter types.
> 
> * I also added javadoc to all public members of NMT
> 
> * I changed all invokers to use NMT where possible, instead of arrays
> of LayoutType
> 
> * I reverted changes in order of import in j.l.ClassLoader; these were
> pushed by mistake (probably as a consequence of some automatic IDE
> reordering). The idea is that we should try not to have deliberate
> difference with upstream, as these will cause merge failures (and
> imports is the #1 reason as to why Java code fails to merge :-))
> 
> 
> Webrev:
> 
> http://cr.openjdk.java.net/~mcimadamore/panama/8215289/
> 
> Maurizio