RFR: 6899049: G1: Clean up code in ptrQueue.[ch]pp and ptrQueue.inline.hpp

Kim Barrett kim.barrett at oracle.com
Tue Nov 3 02:53:35 UTC 2015

> From: thomas.schatzl at oracle.com
> Sent: Monday, November 2, 2015 7:01:47 AM GMT -05:00 US/Canada Eastern

Thanks Thomas.  Responses inline below.

> On Fri, 2015-10-30 at 17:04 -0700, Kim Barrett wrote:
> > Please review this cleanup of various issues in ptrQueue and related
> > classes, e.g. ObjPtrQueue, DirtyCardQueue, and the associated XxxSet
> > classes.  This is in preparation for some structural improvements.
> >
> > [...]
> >
> > CR:
> > https://bugs.openjdk.java.net/browse/JDK-6899049
> >
> > Webrev:
> > http://cr.openjdk.java.net/~kbarrett/6899049/webrev.00/

>  - in DirtyCardQueue::consume()/DCQ::apply_closure_to_buffer()), could
> you add the missing braces in the if clauses?

Sure, though I'm leaving one alone - in apply_closure_to_buffer
  if (cl == NULL) return true;
That's a line that I want to remove entirely in a later change.  If
one doesn't have a closure, one shouldn't be attempting to apply it.

>  - would it be doable to remove the pragma in dirtyCardQueue.cpp (and
> satbQueu.cpp)?

Looks like you've already taken care of this with JDK-8141134.

>  - DirtyCardQueueSet::DirtyCardQueueSet, in the initializer list for
> _shared_dirty_card_queue(), /*perm*/ should read /* permanent */ now.

Good catch.  Similarly for SATBMarkQueueSet.

>  - potentially fix the use of ints for sizes in FreeIdSet too. May be
> handled in a separate CR though.

Not in this CR.

>  - please fix the additional CR in the definition of
> DCQS::get_completed_buffer() after the return value. Maybe also in
> DCQS::apply_closure_to_completed_buffer_helper.

It took me a while to figure out what you were talking about, because
of the different binding for "CR".  Sure.

More information about the hotspot-gc-dev mailing list