[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-hotspot-runtime-dev
Subject:    Re: RFR(S/T): 8230876: baseline cleanups from Async Monitor Deflation v2.0[789]
From:       "Daniel D. Daugherty" <daniel.daugherty () oracle ! com>
Date:       2019-11-19 23:25:34
Message-ID: 9074fd3f-1273-38a4-119c-3b66d736cda4 () oracle ! com
[Download RAW message or body]

On 11/19/19 6:23 PM, David Holmes wrote:
> On 20/11/2019 9:21 am, Daniel D. Daugherty wrote:
>> So I changed the _recursions field from 'intptr_t' -> 'intx' and then
>> I grepped around for uses of the field. One thread on the sweater led
>> to another... It's still a pretty trivial change... especially if you
>> look at the Udiff links...
>>
>> Incremental webrev:
>>
>> http://cr.openjdk.java.net/~dcubed/8230876-webrev/1-for-jdk14.inc/
>
> That all looks fine to me.

Thanks for the very fast re-review!

Dan


>
> Thanks,
> David
> -----
>
>> Full webrev:
>>
>> http://cr.openjdk.java.net/~dcubed/8230876-webrev/1-for-jdk14.full/
>>
>> I'm rerunning Mach5 Tier[1-3] just to be sure...
>>
>> Dan
>>
>>
>> On 11/19/19 9:41 AM, Daniel D. Daugherty wrote:
>>> Hi David,
>>>
>>> Thanks for the quick review!
>>>
>>> On 11/18/19 9:23 PM, David Holmes wrote:
>>>> Hi Dan,
>>>>
>>>> Given:
>>>>
>>>>  volatile intptr_t _recursions;
>>>>
>>>> The change to the print statements to use INTX_FORMAT instead of 
>>>> the existing INTPTR_FORMAT seems a little odd - but obviously you 
>>>> don't want it printed in hex.
>>>
>>> Yup. I first noticed it when I wrote and tested my initial version of
>>> ObjectMonitor::print_debug_style_on() and then I realized that it 
>>> was all
>>> over with other prints of that field.
>>>
>>>
>>>> That seems fine, but can we then make the simple change to redefine 
>>>> _recursions as intx as well - which is a semantic no-op given:
>>>>
>>>> typedef intptr_t  intx;
>>>
>>> No problem. I'll make that change before I push. Not sure why that
>>> didn't occur to me before.
>>>
>>>
>>>> Otherwise all seems okay.
>>>
>>> Thanks! Now, if we can just get a quick review of 
>>> macroAssembler_x86.cpp
>>> from Vladimir K... :-)
>>>
>>> Dan
>>>
>>>
>>>>
>>>> Thanks,
>>>> David
>>>>
>>>> On 19/11/2019 8:23 am, Daniel D. Daugherty wrote:
>>>>> Greetings,
>>>>>
>>>>> I have another round of baseline cleanup changes from the Async 
>>>>> Monitor
>>>>> Deflation project (8153224). Like previous sub-tasks of 8153224, 
>>>>> these
>>>>> changes are small and/or trivial. These changes have previously been
>>>>> reviewed as a (very) small part of 8153224 (CR8/v2.08/11-for-jdk14).
>>>>>
>>>>> Vladimir K., if you could sanity check the cleanups in 
>>>>> macroAssembler_x86.cpp
>>>>> that would be appreciated (only comments were changed). I 
>>>>> recommend the
>>>>> Udiff view...
>>>>>
>>>>> Please see the bug for details about the changes in this webrev:
>>>>>
>>>>>      JDK-8230876 baseline cleanups from Async Monitor Deflation 
>>>>> v2.0[789]
>>>>>      https://bugs.openjdk.java.net/browse/JDK-8230876
>>>>>
>>>>> Here's the webrev URL:
>>>>>
>>>>> http://cr.openjdk.java.net/~dcubed/8230876-webrev/0-for-jdk14/
>>>>>
>>>>> These changes have been included in my recent rounds of Mach5 
>>>>> Tier[1-8]
>>>>> and other associated stress and/or performance testing. I have 
>>>>> also done
>>>>> a Mach5 Tier[1-3] run with just this patch to make sure that I got 
>>>>> all
>>>>> the pieces that are needed.
>>>>>
>>>>> Thanks, in advance, for any comments, questions or suggestions.
>>>>>
>>>>> Dan
>>>
>>>
>>

[prev in list] [next in list] [prev in thread] [next in thread]