RFR (M): 8071278: Fix the closure mess in G1RemSet::refine_card()

Stefan Johansson stefan.johansson at oracle.com
Mon Mar 6 14:36:50 UTC 2017



On 2017-02-24 15:36, Thomas Schatzl wrote:
> Hi Stefan,
>
> On Fri, 2017-02-24 at 14:07 +0100, Stefan Johansson wrote:
>> Thanks for cleaning this up Thomas,
>>
>> On 2017-02-23 12:23, Thomas Schatzl wrote:
>>> Hi all,
>>>
>>>     one line:
>>>
>>> "188 lines changed: 23 ins; 143 del; 22 mod; 3097 unchg"
>>>
>>> This is a rip-out and manual collapsing of needlessly complex code
>>> in the update rs algorithm.
>>>
>>> There are more improvements obvious now in that area (JDK-8071280,
>>> JDK-8162928, JDK-8175554 at least), so I kept this one to the
>>> minimum.
>>>
>>> The change makes the code a bit faster too due to avoiding repeated
>>> null-checks, region-crossing checks etc. etc.
>>>
>>> CR:
>>> https://bugs.openjdk.java.net/browse/JDK-8071278
>>> Webrev:
>>> http://cr.openjdk.java.net/~tschatzl/8071278/webrev/
>> Change looks good in general, just a few small things.
>> ---
>> src/share/vm/gc/g1/g1_specialized_oop_closures.hpp:
>>    45       f(G1UpdateRSOrPushRefOopClosure,_nv)              \
>>
>> Alignment of '\'.
> Fixed.
>
>> ----
>> src/share/vm/gc/g1/g1OopClosures.hpp:
>> 192   bool apply_to_weak_ref_discovered_field() { return true; }
>>
>> Did you do any analysis whether or not this is needed? Or did you
>> just re-add it since FilterOutOfRegionClosure had it? I never figured
>> out a clear answer to that when I was looking at this a while back. I
>> don't see any need to change this right now, just curious if you know
>> why it is needed.
> I did only do cursory analysis about that, but it seems that some code
> (G1CollectedHeap::preserve_cm_referents(), pending list management)
> relies on it, so it needs to be preserved, and so simply added it when
> removing the FilterOutOfRegionClosure you described.
>
> I will think about it again in more detail soonish.
Sounds fair.

Shit it,
StefanJ
> Thanks,
>    Thomas
>



More information about the hotspot-gc-dev mailing list