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