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

[Coleen Phillimore]

Thanks, George!
Coleen

On 12/18/15 3:42 PM, George Triantafillou wrote:
> 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
>>>>
>>>>
>>>
>>
>