RFR: 8146399: Refactor the BlockOffsetTable classes.

Thomas Schatzl thomas.schatzl at oracle.com
Thu Jan 7 13:23:12 UTC 2016


Hi,

On Thu, 2016-01-07 at 12:27 +0100, David Lindholm wrote:
> Hi Mikael,
> 
> Thanks for looking at this! I have fixed your comments:
> 
> http://cr.openjdk.java.net/~david/JDK-8146399/webrev.01/
> 
> 

- the comment for the G1BlockOffsetTable constructor is out of date
(pre-existing).

- I would prefer if in g1BlockOffsetTable.inline.hpp, the definition of
block_start_const()/block_size()/block_at_or_preceding()/forward_to_blo
ck_containing_addr_const() the newline after the return type were
removed.

- in the same file, the arguments of block_at_or_preceding() are not
aligned properly

- in the same file, in forward_to_block_containing_addr_const(), please
add braces for the early exit (line 140) at the if-statement.

- in the same file, forward_to_block_containing() also misses braces in
the first if-statement

- g1BlockOffsetTable.cpp, additional newline in line 203

- in heapRegion.inline.hpp, the change in the name of the
G1OffsetTableContigSpace causes lots of parameter specifications to be
not conforming to code style. Could you look over this file again?

- in case the SA agent is still around, did you check the changes in
vmstructs_g1 with it?

Thanks a lot for the effort.

Thomas




More information about the hotspot-gc-dev mailing list