RFR: JDK-8180193: Make marking bitmap code available to other GCs

Stefan Johansson stefan.johansson at oracle.com
Wed Aug 29 14:32:45 UTC 2018



On 2018-08-29 16:30, Roman Kennke wrote:
> Hi Stefan,
> 
> thanks for reviewing!
> 
>> On 2018-08-28 13:01, Roman Kennke wrote:
>>> Ping? Maybe someone from G1 could review it?
>>>
>>> Thanks, Roman
>>>
>>>
>>>>>> http://cr.openjdk.java.net/~rkennke/JDK-8180193/webrev.02/
>>>>>
>>>>> Looks good to me.
>>>>>
>>>>> Minor nits:
>>>>>
>>>>>    *) I think comma is missing here after "2018" in most files:
>>>>>
>>>>>      2  * Copyright (c) 2017, 2018 Oracle and/or its affiliates. All
>>>>> rights reserved.
>>>>>
>>>>
>>>> Ok, fixed here:
>>>> http://cr.openjdk.java.net/~rkennke/JDK-8180193/webrev.03
>>>>
>> Looks good in general, just a few small comments.
>>
>> ---
>> src/hotspot/share/gc/g1/g1ConcurrentMarkBitMap.hpp
>> src/hotspot/share/gc/g1/g1ConcurrentMarkBitMap.inline.hpp
>>   #include "gc/shared/markBitMap.inline.hpp"
>>
>> Should be after gc/g1/... to have the includes sorted.
>> ---
>> src/hotspot/share/gc/shared/markBitMap.inline.hpp
>>    28 #include "gc/shared/markBitMap.hpp"
>>    29 #include "gc/shared/collectedHeap.hpp"
>>
>> Switch these two lines.
>> ---
> 
> Ok, fixed.
> 
>> Regarding the check_mark change I'm not to fond of removing the "exact"
>> check. An alternative to adding a virtual is_in_exact() to CollectedHeap
>> would be to make check_mark itself virtual and let G1 implement the
>> exact check. What do you think about that?
> 
> Yes, that's a good idea. It does not play well with that method being
> inline, but it's assert-only, so who needs this inlined?
I agree, I see no need to have it inlined.
> 
> Incremental:
> http://cr.openjdk.java.net/~rkennke/JDK-8180193/webrev.04.diff/
> Full:
> http://cr.openjdk.java.net/~rkennke/JDK-8180193/webrev.04/
> 
> What do you think?
>
Looks good,
StefanJ

> Thanks,
> Roman
> 



More information about the hotspot-gc-dev mailing list