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

Thomas Schatzl thomas.schatzl at oracle.com
Wed Nov 6 09:22:44 UTC 2019


Hi Stefan,

   thanks for your review.

On 05.11.19 12:35, Stefan Johansson wrote:
> 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.

Good catch. Changed in

http://cr.openjdk.java.net/~tschatzl/8189737/webrev.0_to_1/ (diff)
http://cr.openjdk.java.net/~tschatzl/8189737/webrev.1 (full)

I ran hs-tier6 (containing G1 SA tests) and jtreg/serviceability/sa 
tests with that with no issue.

Thanks,
   Thomas



More information about the hotspot-gc-dev mailing list