RFR (s): 8026849 - Fix typos in the GC code, part 2

Stefan Karlsson stefan.karlsson at oracle.com
Thu Jan 30 12:22:14 UTC 2014


On 2014-01-30 13:20, Jesper Wilhelmsson wrote:
> Hi again,
>
> I got comments from several people that the comma-fix described in 
> 8023899 really should be fixed, so I'm reopening the bug. I will not 
> fix it in this change but leave the discussion for a different RFR.
>
> There are a few other changes left in the patch though, so I'm still 
> looking for reviewers :-)
>
> The latest webrev is:
> http://cr.openjdk.java.net/~jwilhelm/8026849/webrev.1/

Looks good.

StefanK

>
> Thanks,
> /Jesper
>
>
> Jesper Wilhelmsson skrev 30/1/14 12:04 PM:
>> Hi Kirk,
>>
>> Your opinion is as important as anyone else's. I have removed the 
>> change in
>> timer.cpp and updated the webrev. I also closed the bug as WNF.
>>
>> Cheers,
>> /Jesper
>>
>>
>> Kirk Pepperdine skrev 29/1/14 4:22 PM:
>>> Hi Jesper,
>>>
>>> Well, I’m not a reviewer so you can easily ignore my plea for 
>>> stability in the
>>> GC logs but I appreciate the consideration.
>>>
>>> Regards,
>>> Kirk
>>>
>>> On Jan 29, 2014, at 3:01 PM, Jesper Wilhelmsson
>>> <jesper.wilhelmsson at oracle.com> wrote:
>>>
>>>> Hi Kirk,
>>>>
>>>> Thanks for looking at this!
>>>>
>>>> As I wrote in bug 8023899, I agree with your assessment that it may 
>>>> not be
>>>> worth the effort to fix the comma. I won't fight for fixing it, I 
>>>> just wanted
>>>> to get a few official comments as motivation to close the bug as 
>>>> WNF. :-)
>>>>
>>>> Thanks,
>>>> /Jesper
>>>>
>>>>
>>>> Kirk Pepperdine skrev 29/1/14 8:54 AM:
>>>>>
>>>>> On Jan 29, 2014, at 12:29 AM, Jesper Wilhelmsson 
>>>>> <jesper.wilhelmsson at oracle.com
>>>>> <mailto:jesper.wilhelmsson at oracle.com>> wrote:
>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> Could I have a couple of reviews for this small patch?
>>>>>>
>>>>>> These are fixes for a couple of typos that I didn't include in 
>>>>>> the big fix
>>>>>> recently pushed. They were not included since they do not reside 
>>>>>> in comments
>>>>>> but rather assert messages, flag descriptions and some verbose 
>>>>>> messages.
>>>>>>
>>>>>> Bug:
>>>>>> https://bugs.openjdk.java.net/browse/JDK-8026849
>>>>>>
>>>>>> Webrev:
>>>>>> http://cr.openjdk.java.net/~jwilhelm/8026849/webrev/
>>>>>>
>>>>>>
>>>>>> This patch also contains a fix for 8023899 - Typo in TraceCPUTime 
>>>>>> message.
>>>>>> https://bugs.openjdk.java.net/browse/JDK-8023899
>>>>>
>>>>> -        _logfile->print(" [Times: user=%3.2f sys=%3.2f, 
>>>>> real=%3.2f secs] ",
>>>>> +        _logfile->print(" [Times: user=%3.2f sys=%3.2f real=%3.2f 
>>>>> secs] ",
>>>>>             user_secs, system_secs, real_secs);
>>>>>
>>>>>
>>>>>
>>>>> caveat, not a official reviewer. I looked at all the typo fixes 
>>>>> and they all
>>>>> look good. The only thing I can comment on is that I appreciate 
>>>>> what you’re
>>>>> trying to do here but I don’t really care if there is a comma or a 
>>>>> space
>>>>> between
>>>>> the entries as I don’t read these files but my parser does read 
>>>>> them and it
>>>>> cares. Yeah it’s a trivial fix to the parser for this one but when 
>>>>> I look at
>>>>> all
>>>>> the spaces, commas, ’s’es and other trivial formatting changes 
>>>>> that creep in
>>>>> over time it starts to get really messy. Can we please just leave 
>>>>> this one out?
>>>>>
>>>>> Kind regards,
>>>>> Kirk
>>>>>
>>>




More information about the hotspot-gc-dev mailing list