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