RFR (M): 8189737: Make HeapRegion not derive from Space

Stefan Johansson stefan.johansson at oracle.com
Tue Nov 5 11:35:38 UTC 2019


Hi Thomas,

On 2019-11-04 17:40, Thomas Schatzl wrote:
> Hi,
> 
> On 01.11.19 00:20, Thomas Schatzl wrote:
>> Hi Kim,
>>
>>    thanks for your review.
>>
>> On Thu, 2019-10-31 at 18:12 -0400, Kim Barrett wrote:
>>>> On Oct 31, 2019, at 9:43 AM, Thomas Schatzl <
>>>> thomas.schatzl at oracle.com> wrote:
>>>>
>>>> Hi all,
>>>>
>>>>   can I get reviews for this refactoring that removes the
>>>> inheritance of HeapRegion from Space?
>>>>
>>>>
>> [...]
>>>> CR:
>>>> https://bugs.openjdk.java.net/browse/JDK-8189737
>>>> Webrev:
>>>> http://cr.openjdk.java.net/~tschatzl/8189737/webrev/

I think this looks really good in general, so nice to get rid of this 
inheritance.

I have one question/comment though. I took a look at the HeapRegion 
implementation in the SA and there it still extends CompactibleSpace 
(which extends Space), I think we should remove this and add _bottom and 
_top for HeapRegion in vmStructs_g1.hpp. This also has the effect that 
the PrintRegionClosure needs to be updated to not depend on space 
either. It is only used by G1 so I think it should simply be moved from 
shared to g1 and changed to not depend on space.

Thanks,
Stefan

>>>> Testing:
>>>> hs-tier-1-5
>>>>
>>>> Thanks,
>>>>   Thomas
> [..]
> 
>>>
>>> -------------------------------------------------------------------
>>> -----------
>>>
>>> Looks good.
>>>
>>> I don't need a new webrev for the parameter list indentation fix.
>>>
>>
>> I will update the webrev later in place.
> 
> Done. Thanks for your review.
> 
> Thomas
> 



More information about the hotspot-gc-dev mailing list