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

[Bengt Rutisson]

> 8 dec 2014 kl. 16:25 skrev Kim Barrett <kim.barrett at oracle.com>:
> 
>> 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.

Thanks for the review, Kim!

Bengt

>