RFR (M): 8066780, 8066781, 8066782: Cleanup of duplicated code in TenuredGeneration and ConcurrentMarkSweepGeneration

Kim Barrett kim.barrett at oracle.com
Mon Dec 8 15:25:36 UTC 2014


On Dec 8, 2014, at 10:20 AM, Bengt Rutisson <bengt.rutisson at oracle.com> wrote:
> 
>>> src/share/vm/memory/tenuredGeneration.inline.hpp
>>>   44   if (addr < _the_space->top()) return oop(addr)->size();
>>>   45   else {
>>>   46     assert(addr == _the_space->top(), "non-block head arg to block_size");
>>>   47     return _the_space->end() - _the_space->top();
>>>   48   }
>>> 
>>> I'm not sure I've ever before seen that formatting style for an "if",
>>> and I'm finding it painful to parse.  I also think it violates the
>>> Hotspot StyleGuide, e.g.
>>>   Use braces around substatements. (Relaxable for extremely simple
>>>   substatements on the same line.)
>>> Would you mind adding braces, and a line break before the first
>>> return, since you are touching these lines anyway?
>> 
>> Quite agree. Horrible formatting! I changed it to:
>> 
>> size_t TenuredGeneration::block_size(const HeapWord* addr) const {
>>  if (addr < _the_space->top()) {
>>    return oop(addr)->size();
>>  }
>>  else {
>>    assert(addr == _the_space->top(), "non-block head arg to block_size");
>>    return _the_space->end() - _the_space->top();
>>  }
>> }
> 
> A small update. Stefan J. just pointed out that the else statement is a on a separate line. I prefer to have it on the same line as the "}".  So, I've updated my change to use:
> 
> 
> size_t TenuredGeneration::block_size(const HeapWord* addr) const {
>  if (addr < _the_space->top()) {
>    return oop(addr)->size();
>  } else {
>    assert(addr == _the_space->top(), "non-block head arg to block_size");
>    return _the_space->end() - _the_space->top();
>  }
> }

Thanks.

Looks good.




More information about the hotspot-gc-dev mailing list