RFR 8080232: Eagerly promote objects in nmethod to reduce scanning in ParNew

[Tony Printezis]

Carsten,

I think this looks OK. A few suggestions:

codeCache.{hpp,cpp}:

(Unrelated to your change) Since you’re at it, could you add a big warning to the declaration of scavenge_root_nmethods_do() that, if the caller is moving objects, it has to move them promptly when the closure is invoked and not enqueue them for later processing? All the uses of this are correct as far as I can see. For example, PS does push on the taskqueues oop*’s for later processing but in the closure that it passes to scavenge_root_nmethods_do(), it copies (in fact: it promotes) the objects immediately. If the copy is postponed, fix_oop_relocations() will pick up the wrong object(s). It’s a subtle point that I don’t think it’s properly documented. It’d be good to document it somewhere. Or maybe CodeBlobToOopClosure is a good place for this (as that’s the closure that calls fix_oop_relocations()).

Given that scavenge_root_nmethods() now prunes the list, could you add a comment to the declaration of prune_scavenge_root_nmethods() to indicate where it is used (i.e., to prune the list post full compaction I guess?). In fact, is prune_scavenge_root_nmethods() called from outside the CodeCache now? If not, maybe make it private?

The semantics of drop_scavenge_root_nmethod(), the way you changed it, are a bit confused. prev == NULL might mean “we don’t know the prev of nm, look for it” or “nm is the head”. I’d suggest leave drop_scavenge_root_nmethod() as is (semantic-wise), introduce a new (private) method (like: unlink_scavenge_root_nmethod(nm, prev)) which does the unlinking, and call it from drop_scavenge_root_nmethod() and scavenge_root_nmethods_do().

Tony
On February 16, 2016 at 9:54:56 PM, Carsten Varming (varming at gmail.com) wrote:

Dear Jon,

Oops, I don't know what I was thinking when I found that ticket. I'll try to put it back in its old state. I filed a new ticket: JDK-8150013.

I folded the pruning of the scavengable nmethod list into CodeCache::scavenge_root_nmethods_do, thus removing the need for a separate pruning step after the young collection.

You can find the new webrev here: http://cr.openjdk.java.net/~cvarming/1/webrev/

Carsten



On Tue, Feb 16, 2016 at 6:53 PM, Jon Masamitsu <jon.masamitsu at oracle.com> wrote:
Carsten,

This looks like a good change, but 8080232 describes a different problem.
8080232 proposed early promotion for objects in nmethods.  This change
removes nmethods from a list when it is determined that they don't refer to
scavengable objects.  Can you create a new CR for this?

Jon


On 02/15/2016 12:53 PM, Carsten Varming wrote:
Dear GC members,

I would like to contribute a fix for JDK-8080232. The bug is marked as resolved because it was withdrawn. The fix is simple, so I would like to reopen the ticket. At Twitter we have seen the time to fix relocations after a ParNew cycle drop by three to four orders of magnitude with this patch.

I have put a webrev here: http://cr.openjdk.java.net/~cvarming/prune_scavengable_nmethods/0/webrev/

I will need a sponsor.

Carsten


-----

Tony Printezis | JVM/GC Engineer / VM Team | Twitter

@TonyPrintezis
tprintezis at twitter.com

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