RFR: 8146399: Refactor the BlockOffsetTable classes.

Mikael Gerdin mikael.gerdin at oracle.com
Thu Jan 7 10:08:31 UTC 2016


Hi David,

Overall a nice cleanup!
Good stuff!
I have some small comments, see below.

On 2016-01-04 15:07, David Lindholm wrote:
> Hi,
>
> Please review the following patch which refactors the Block Offset Table
> classes in G1.
>
> Right now there are 3 classes which are responsible for a part of the
> Block Offset Table. These are G1BlockOffsetArrayContigSpace, which
> inherits from G1BlockOffsetArray which inherits from G1BlockOffsetTable.
> I have merged these 3 classes into a new one called
> G1BlockOffsetTablePart, removing some dead code.
>
> The class responsible for the whole BOT has been renamed from
> G1BlockOffsetSharedArray to G1BlockOffsetTable.
>
> Also, the class called G1OffsetTableContigSpace has been renamed to
> G1ContiguousSpace.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8146399
> Webrev: http://cr.openjdk.java.net/~david/JDK-8146399/webrev.00/

g1BlockOffsetTable.hpp:

In the comment before G1BlockOffsetTable it says:
// Each G1BlockOffsetTablePart is owned by a Space.
Should that be narrowed to say that it's owned by a G1ContiguousSpace?


heapRegion.cpp:

in HeapRegion::HeapRegion
                        G1BlockOffsetTable* sharedOffsetArray,
while you're changing the type of the parameter, can you also rename it 
to match the snake_case style of the surrounding code?

in G1ContiguousSpace::G1ContiguousSpace
Can you remove the newline after ::?


heapRegionRemSet.cpp:

in HeapRegionRemSet::print
Can you change the line you edited
       G1CollectedHeap::heap()->bot()->address_for_index(card_index);
to use _bot instead of getting it through the heap?

/Mikael

>
> Testing: JPRT and vm.gc testlist.
>
>
> Thanks,
> David




More information about the hotspot-gc-dev mailing list