review request (M) - 8030646: track cset membership in one place (original) (raw)

John Coomes John.Coomes at oracle.com
Wed Jan 15 19:48:38 UTC 2014

Bengt Rutisson (bengt.rutisson at oracle.com) wrote:

Hi John, 8030647-cset-rename-fast-test 8030648-cset-move-methods 8030649-cset-oop-in-cset 8030650-cset-rm-length These all look good. No comments. 8030651-cset-unify-obj-in-cs This is a nice cleanup. Thanks for fixing this!

Hi Bengt,

Thanks for looking at it. I'm just getting back to this after the holidays and some sick time.

I like the division between incset() and oopincset(). Have you checked if there are more occurrences of incset() that could be replaced by oopincset()? It looks to me like these incset() could be changed to oopincset() in these two methods:

G1CollectedHeap::handleevacuationfailurepar() G1ParScanThreadState::verifyref()

The one in verify_ref(oop* ref) is already oop_in_cset(), unless I'm misunderstanding you. But I missed the two in handle_evacuation_failure_par(); I'll change them, e.g.:

-    assert(old == forward_ptr || !in_cset(forward_ptr),
+    assert(old == forward_ptr || !oop_in_cset(forward_ptr),

And I think G1STWIsAliveClosure::doobjectb() could also use oopincset() too.

That one is harder to be certain about. I'll try some tests with it, but may want to defer.

-John

On 2013-12-18 07:00, John Coomes wrote: > Please review a set of changes to eliminate redundant cset membership > tracking in g1: > > http://cr.openjdk.java.net/~jcoomes/8030646-cset-unify-all.html > > -John

-- John Coomes Oracle, MS USCA22-3?? john.coomes at oracle.com 4220 Network Circle 408-276-7048 Santa Clara, CA 95054-1778 *** Support GreenPeace and we'll all breathe easier. ***

More information about the hotspot-gc-dev mailing list