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

[John Coomes]

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 in_cset() and oop_in_cset(). Have you 
> checked if there are more occurrences of in_cset() that could be 
> replaced by oop_in_cset()? It looks to me like these in_cset() could be 
> changed to oop_in_cset() in these two methods:
> 
> G1CollectedHeap::handle_evacuation_failure_par()
> G1ParScanThreadState::verify_ref()

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::do_object_b() could also use 
> oop_in_cset() 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. ***