RFR: 8146399: Refactor the BlockOffsetTable classes.
Jesper Wilhelmsson
jesper.wilhelmsson at oracle.com
Thu Jan 7 13:21:45 UTC 2016
Looks good!
/Jesper
Den 7/1/16 kl. 12:27, skrev David Lindholm:
> Hi Mikael,
>
> Thanks for looking at this! I have fixed your comments:
>
> http://cr.openjdk.java.net/~david/JDK-8146399/webrev.01/
>
>
> Thanks,
> David
>
> 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:
>>> 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