8034761: Remove the do_code_roots parameter from process_strong_roots

Mikael Gerdin mikael.gerdin at oracle.com
Thu Feb 13 08:30:41 UTC 2014


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.

/Mikael

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




More information about the hotspot-gc-dev mailing list