Review Request (M) 8032379: Remove the is_scavenging flag to process_strong_roots

[Jon Masamitsu]

Mikael,

http://cr.openjdk.java.net/~mgerdin/8032379/webrev.0/src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepGeneration.cpp.frames.html

I don't mean to expand this task but the comment after the parameter
"do_code_roots" caught my eye.

> 3736       gch->gen_process_strong_roots(_cmsGen->level(),
> 3737                                     true,   // younger gens are roots
> 3738                                     true,   // activate StrongRootsScope
> 3739                                     SharedHeap::ScanningOption(roots_scanning_options()),
> 3740                                     &notOlder,
> 3741                                     true,// walk all of code cache if (so & SO_AllCodeCache)
> 3742                                     NULL,
> 3743                                     &klass_closure);

Did you consider whether the "do_code_roots" is needed any more?

http://cr.openjdk.java.net/~mgerdin/8032379/webrev.0/src/share/vm/memory/sharedHeap.cpp.frames.html

>   208     if (so & SO_ScavengeCodeCache) {
>   209       assert(code_roots != NULL, "must supply closure for code cache");
>   210
>   211       // We only visit parts of the CodeCache when scavenging.
>   212       CodeCache::scavenge_root_nmethods_do(code_roots);
>   213     }
>   214     if (so & SO_AllCodeCache) {
>   215       assert(code_roots != NULL, "must supply closure for code cache");
>   216
>   217       // CMSCollector uses this to do intermediate-strength collections.
>   218       // We scan the entire code cache, since CodeCache::do_unloading is not called.
>   219       CodeCache::blobs_do(code_roots);
>   220     }

The new code allows for scavenge_root_nmethods_do() and blobs_do() both 
to be called
(depending on the contents of "so").  That could not happen with the old 
code.  Is that
OK?  I didn't read enough code to know if blobs_do would also process 
the same roots
again as scavenge_root_nmethods_do().

That's all.  Rest looks good.  Nice change.

Jon





On 1/21/2014 5:13 AM, Mikael Gerdin wrote:
> Hi all,
>
> as a part of implementing JEP-156 (G1 class unloading) we are doing some
> cleanups in order to refactor the strong root processing code.
>
> Currently there are two "dimensions" in which SharedHeap::process_strong_roots
> is configured. One is the ScanOption enum, the other is the is_scavenging
> booelan. The semantic meaning of is_scavenging can easily be folded into the
> ScanOption enum by:
>
> * Introducing a SO_AllCodeCache/SO_ScavengeCodeCache distinction (in a way
> similar to SO_AllClasses/SO_SystemClasses)
> * Noting that passing a CLD closure to Threads::oops_do is only needed when we
> want to determine the precise liveness of classes, this is already signaled by
> SO_SystemClasses.
>
> Bug link: https://bugs.openjdk.java.net/browse/JDK-8032379
> Webrev: http://cr.openjdk.java.net/~mgerdin/8032379/webrev.0
>
> Thanks
> /Mikael

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20140122/8f640a67/attachment.htm>