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