RFR: 8234796: Refactor Handshake::execute to take a HandshakeOperation

Robbin Ehn robbin.ehn at oracle.com
Tue Dec 3 08:59:38 UTC 2019


Thanks Coleen!

/Robbin

On 12/2/19 6:28 PM, coleen.phillimore at oracle.com wrote:
> v4 looks good to me.  A lot better than v0.
> 
> http://cr.openjdk.java.net/~rehn/8234796/v4/full/webrev/src/hotspot/share/runtime/deoptimization.cpp.udiff.html 
> 
> 
> Thank you for changing these names from TC.
> 
> Thanks also for moving ThreadClosure to iterator.hpp because that's where I 
> would have looked for it first.
> 
> Coleen
> 
> On 11/29/19 6:52 AM, Robbin Ehn wrote:
>> Hi, Shenandoah just added a handshake, here is the additional fix.
>>
>> http://cr.openjdk.java.net/~rehn/8234796/v4/full/
>> http://cr.openjdk.java.net/~rehn/8234796/v4/inc/
>>
>> Thanks, Robbin
>>
>> On 11/28/19 3:29 PM, Robbin Ehn wrote:
>>> Thanks David!
>>>
>>> Since I had no compile issues, fixing includes for the ThreadClosure move 
>>> slipped my mind.
>>>
>>> Inc:
>>> http://cr.openjdk.java.net/~rehn/8234796/v3/inc/webrev/
>>> Full:
>>> http://cr.openjdk.java.net/~rehn/8234796/v3/full/webrev/
>>>
>>> Built fastdebug and release for x64 win/lin/osx, aarch64, ppc, solaris-sprac.
>>> And without precompiled header locally.
>>> arm32 and x86 have prior build issues and do not compile.
>>>
>>> Thanks, Robbin
>>>
>>> On 2019-11-28 07:21, David Holmes wrote:
>>>> Hi Robbin,
>>>>
>>>> On 28/11/2019 1:25 am, Robbin Ehn wrote:
>>>>> Hi all, please review.
>>>>>
>>>>> Here is the result after Per's suggestion:
>>>>> http://cr.openjdk.java.net/~rehn/8234796/v2/full/webrev/index.html
>>>>> (incremental made no sense)
>>>>>
>>>>> Due to circular dependency between thread.hpp and handshake.hpp, I moved 
>>>>> the ThreadClosure to iterator.hpp, as was suggested offline.
>>>>
>>>> That all looks good to me! Thanks for splitting these up.
>>>>
>>>> Thanks,
>>>> David
>>>> -----
>>>>
>>>>> Passes t1-3
>>>>>
>>>>> Thanks, Robbin
>>>>>
>>>>> On 11/26/19 2:07 PM, Robbin Ehn wrote:
>>>>>> Hi all, please review.
>>>>>>
>>>>>> Issue:
>>>>>> https://bugs.openjdk.java.net/browse/JDK-8234796
>>>>>> Code:
>>>>>> http://cr.openjdk.java.net/~rehn/8234796/full/webrev/
>>>>>>
>>>>>> The handshake code needs more information about the handshake operation.
>>>>>> We change type from ThreadClosure to HandshakeOperation in 
>>>>>> Handshake::execute.
>>>>>> This enables us to add more details to the HandshakeOperation as needed 
>>>>>> going forward.
>>>>>>
>>>>>> Tested t1 and t1-3 together with the logging improvements in 8234742.
>>>>>>
>>>>>> It was requested that "HandshakeOperation()" would take the name instead 
>>>>>> having "virtual const char* name();". Which is in this patch.
>>>>>>
>>>>>> Thanks, Robbin
> 


More information about the hotspot-dev mailing list