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

Jon Masamitsu jon.masamitsu at oracle.com
Wed Jan 22 15:10:33 UTC 2014

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->genprocessstrongroots(cmsGen->level(), 3737 true, // younger gens are roots 3738 true, // activate StrongRootsScope 3739 SharedHeap::ScanningOption(rootsscanningoptions()), 3740 &notOlder, 3741 true,// walk all of code cache if (so & SOAllCodeCache) 3742 NULL, 3743 &klassclosure);

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 & SOScavengeCodeCache) { 209 assert(coderoots != NULL, "must supply closure for code cache"); 210 211 // We only visit parts of the CodeCache when scavenging. 212 CodeCache::scavengerootnmethodsdo(coderoots); 213 } 214 if (so & SOAllCodeCache) { 215 assert(coderoots != 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::dounloading is not called. 219 CodeCache::blobsdo(coderoots); 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::processstrongroots is configured. One is the ScanOption enum, the other is the isscavenging booelan. The semantic meaning of isscavenging can easily be folded into the ScanOption enum by: * Introducing a SOAllCodeCache/SOScavengeCodeCache distinction (in a way similar to SOAllClasses/SOSystemClasses) * Noting that passing a CLD closure to Threads::oopsdo is only needed when we want to determine the precise liveness of classes, this is already signaled by SOSystemClasses. 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>

More information about the hotspot-gc-dev mailing list