RFR: 8146399: Refactor the BlockOffsetTable classes.
david.lindholm at oracle.com
Thu Jan 7 11:27:46 UTC 2016
Thanks for looking at this! I have fixed your comments:
On 2016-01-07 11:08, Mikael Gerdin wrote:
> Hi David,
> Overall a nice cleanup!
> Good stuff!
> I have some small comments, see below.
> On 2016-01-04 15:07, David Lindholm wrote:
>> 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
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8146399
>> Webrev: http://cr.openjdk.java.net/~david/JDK-8146399/webrev.00/
> 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?
> 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 ::?
> in HeapRegionRemSet::print
> Can you change the line you edited
> to use _bot instead of getting it through the heap?
>> Testing: JPRT and vm.gc testlist.
More information about the hotspot-gc-dev