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

[Coleen Phillimore]

Hi, Can I get another review?  I tried to be minimal so it'd be easy to 
review.

thanks,
Coleen

cc. Kim who may not be on hotspot-runtime-dev.

On 12/18/15 7:38 AM, harold seigel wrote:
> 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
>>
>>
>