8034761: Remove the do_code_roots parameter from process_strong_roots

Stefan Karlsson stefan.karlsson at oracle.com
Thu Feb 13 08:49:01 UTC 2014


Mikael,

On 13/02/14 09:30, Mikael Gerdin wrote:
> Stefan,
>
> On Wednesday 12 February 2014 10.43.33 Stefan Karlsson wrote:
>> Hi all,
>>
>> Please, review this patch to remove the do_code_roots parameter from
>> SharedHeap::process_strong_roots.
>>
>> The changes done are:
>> - Change the code to rely on the ScannningOption so parameter instead of
>> do_code_roots.
>> - Change GenMarkSweep and G1MarkSweep to adjust the code roots with the
>> help of process_strong_roots instead of doing it as a separate phase
>> after process_strong_roots.
>> - Removed the unused FalseClosure.
>>
>> After this patch the adjust phase of the GenMarkSweep and G1MarkSweep
>> will use the generic code in process_strong_roots, which mark/claim the
>> nmethods before they are processed. Before the patch these two Serial
>> Old GC adjust phases skipped the mark/claim part. No noticeable Serial
>> Old GC time increases were found when this patch was performance tested.
>>
>> This cleanup is needed/wanted for G1 Class Unloading.
>>
>> Webrev:
>> http://cr.openjdk.java.net/~stefank/8034761/webrev.00/
> I think the change is good but I have a comment about the changes to G1's
> verification code:
>
> 3399     // We apply the relevant closures to all the oops in the
> 3400     // system dictionary, the string table and the code cache.
> 3401     const int so = SO_AllClasses | SO_Strings | SO_AllCodeCache;
> 3402
>
> Shouldn't you remove SO_AllCodeCache here since you plan to go through the
> entire code cache just below?
> Otherwise you're just going to do all the nmethod liveness verification work
> twice since G1VerifyCodeRootOopClosure only adds verification (compared to
> VerifyRootsClosure) if the non-default flag +G1VerifyHeapRegionCodeRoots is
> set.

Yes, you're right.

http://cr.openjdk.java.net/~stefank/8034761/webrev.01.delta
http://cr.openjdk.java.net/~stefank/8034761/webrev.01

thanks for the review,
StefanK

>
> /Mikael
>
>> RFE:
>> https://bugs.openjdk.java.net/browse/JDK-8034761
>>
>> thanks,
>> StefanK




More information about the hotspot-gc-dev mailing list