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

[George Triantafillou]

Hi Coleen,

Your changes look good.

-George

On 12/18/2015 2:48 PM, Coleen Phillimore wrote:
>
> 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
>>>
>>>
>>
>