RFR (s): 8147003: Move BubbleUpRef test into CMS directory

Bengt Rutisson bengt.rutisson at oracle.com
Wed Feb 3 15:17:15 UTC 2016


Hi Dima,

Thanks for you comments. Makes sense. And thanks for providing the diff.

Reviewed.

Bengt


On 2016-02-03 15:38, Dmitry Fazunenko wrote:
> Hi Bengt,
>
> Thanks a lot for review and the comments provided.
>
> On 03.02.2016 17:02, Bengt Rutisson wrote:
>>
>> Hi Dima,
>>
>> On 2016-02-03 14:24, Dmitry Fazunenko wrote:
>>> Hi all,
>>>
>>> May I have a couple of reviews of a simple change:
>>>
>>> http://cr.openjdk.java.net/~dfazunen/8147003/webrev.00/
>>> https://bugs.openjdk.java.net/browse/JDK-8147003
>>>
>>> Background:
>>> This test is not new, I just took the source of existing closed 
>>> test, performed some cosmetic changes and put in open.
>>> The original logic has preserved unchanged.
>>
>> I'm fine with pushing this, but I would have preferred if the test 
>> would just have been moved and the cosmetic changes would have been 
>> done as a separate change. Not all of the cosmetic changes are 
>> obvious improvements. I am for example not sure why you wanted to 
>> remove the help text that was logged if the test was started with the 
>> wrong arguments.
>
> I moved the help text from the code into method description. It's very 
> unlikely that code will be ever executed because the arguments are 
> hardcoded.
> So, it's better to have test argument description in the comments.
>
>>
>> As I said, unless anybody else thinks otherwise I am ok with this 
>> change. But I would prefer if future changes like this was separated 
>> in to a move that is as clean as possible and then a separate change 
>> to clean the code up.
>
> I was thinking about making the change in multiple steps, but decided 
> that it would be over complicated for such a simple thing.
> Okay, I got your point for the future.
>
> I attached the diff between old and new version. It's hard to read, 
> because of indent changes, so I didn't attach it earlier.
>
> Thanks,
> Dima
>
>
>>
>> Thanks,
>> Bengt
>>
>>>
>>> Thanks,
>>> Dima
>>
>



More information about the hotspot-gc-dev mailing list