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

Robbin Ehn robbin.ehn at oracle.com
Fri Nov 29 11:52:49 UTC 2019


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