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

[Thomas Schatzl]

Hi Kim,

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.
> 
> Remove unused DirtyCardQueue::set_buf() and PtrQueue::index_to_byte_index().
> 
> Renamed PtrQueue::_perm to _permanent, to match reader function.
> 
> Changed iteration over PtrQueue buffer to use element indexing rather
> than bytes converted to elements on each iteration.  For SATB buffer
> filtering, directly use pointer-based iteration.
> 
> Fixed index types for byte_index_to_index to be size_t rather than
> int, eliminating the need for a bunch of conversion casts.
> 
> In PtrQueue, use sizeof(void*) rather than oopSize for byte
> adjustments. This class shouldn't know anything about oops.
> 
> More restrictive access to various members, including non-public
> constructor and destructor for PtrQueue and PtrQueueSet base classes.
> 
> In queue constructors, fixed type of associated set to match the queue
> type, e.g. DirtyCardQueue requires a DirtyCardQueueSet rather than
> just any old PtrQueueSet.
> 
> debug_only => DEBUG_ONLY.
> 
> CR:
> https://bugs.openjdk.java.net/browse/JDK-6899049
> 
> Webrev:
> http://cr.openjdk.java.net/~kbarrett/6899049/webrev.00/
> 
> Testing:
> JPRT, Aurora ad hoc GC/Runtime Nightly

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

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

I filed JDK-8141134 for the entire gc code base, but it would be an
opportunity to do that here for the touched files too.

A JPRT run with all similar g1 code pragmas removed just ran through
here. Otherwise this can wait for that mentioned CR.

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

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

 - 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.

Looks good otherwise.

Thanks,
  Thomas


 -