RFR 8144256: compiler/uncommontrap/TestStackBangRbp.java crashes VM on Solaris

[harold seigel]

Hi Coleen,

The changes look good.

Harold

On 12/17/2015 12:22 PM, Coleen Phillimore wrote:
> Summary: Take out inlining of methodHandle copy constructors and 
> destructors
>
> Also made a few less copy constructor calls in the verifier.  I looked 
> at the generated .s file for before/after 
> ClassVerifier::verify_method() call and before has 61 copy 
> constructors and 569 destructors.  After has 8 copy constructors and 
> 516 destructors.  The destructor calls are for the CHECK exits in 
> verify_method() to destruct the correctly copied methodHandle object 
> in BytecodeStream.   It would be _really nice_ to do some more 
> refactoring of verify_method() so that more bytecodes call out to a 
> separate verify_xx, like the dup2_xwhatever ones for example.
>
> I ran this through RBT quick (on all platforms), the test case that 
> failed with product build, and tested with refworkload that there's no 
> performance regression.  I also tested java -Xverify:all -version with 
> and without this change with no difference in performance.
>
> Note also, that the original failure was due to a solaris x86 c++ 
> compiler bug that will be fixed in the next version.  The bug looks 
> like the solaris register allocator decided to bail on verify_method 
> (still trying to get the bug report from the compiler team).   Even 
> so, these methodHandle functions make calls and are too large to have 
> inlined.  With the elimination of copy constructor calls and 
> associated destructor calls in the change that passes const 
> methodHandle&, this won't have any performance impact (and may improve 
> generated code).  Also, methodHandles are different than oop Handles 
> and maybe we should have changed their name to methodPtr or 
> methodRegistrar or something but I didn't like any of the ideas for 
> new names.   Kim suggested further improvements to methodHandles 
> offline, like not saving Thread* but we need it for construction and 
> destruction so might as well save it, now that we pass these by const 
> reference.
>
> open webrev at http://cr.openjdk.java.net/~coleenp/8144256/
> bug link https://bugs.openjdk.java.net/browse/JDK-8144256
>
> Thanks,
> Coleen
>
>