RFR: 8135253: Add push method to CollectionSetChooser

[Erik Helin]

And here is a link to the full webrev:
http://cr.openjdk.java.net/~ehelin/8135253/webrev.01/

Thanks,
Erik

On 2015-09-09, Erik Helin wrote:
> On 2015-09-09, Mikael Gerdin wrote:
> > Hi Erik,
> 
> Hi Mikael, thanks for reviewing!
> 
> > On 2015-09-09 13:50, Erik Helin wrote:
> > >Hi all,
> > >
> > >this patch adds a new `push` method to CollectionSetChooser so that
> > >regions can be put back at the front of the list. I've split the
> > >patches into three parts to ease reviewing, but I intend to push this as
> > >one commit. While adding the method I also renamed two variables and one
> > >method to make it easier to understand that the CollectionSetChooser
> > >essentially is a stack.
> > >
> > >Part 0: Rename _curr_index to _front and _length to _end:
> > >http://cr.openjdk.java.net/~ehelin/8135253/webrev.00-0/
> > >
> > >Part 1: Rename remove_and_move_to_next() to pop()
> > >http://cr.openjdk.java.net/~ehelin/8135253/webrev.00-1/
> > 
> > Given
> > HeapRegion* hr = regions_at(_front);
> > the assert
> > assert(regions_at(_front) == hr, "pre-condition");
> > can probably be removed
> 
> Fixed.
> 
> > We ususally don't cast unused return values to void.
> > (void) cset_chooser->pop(); // already have region via peek()
> 
> Fixed.
> 
> Please see new incremental webrev at:
> http://cr.openjdk.java.net/~ehelin/8135253/webrev.01-1/
> 
> > >
> > >Part 2: Add the push() method
> > >http://cr.openjdk.java.net/~ehelin/8135253/webrev.00-2/
> > 
> > pop does _front += 1, the following seems needlessly verbose:
> > 
> >  165   _front = _front - 1;
> 
> To make the style similar to the `add_region` method a couple of lines
> above, I changed the expression to `_front--`. Please see new
> incremental webrev at:
> http://cr.openjdk.java.net/~ehelin/8135253/webrev.01-2/
> 
> Thanks,
> Erik
> 
> > /Mikael
> > 
> > >
> > >Enhancement:
> > >https://bugs.openjdk.java.net/browse/JDK-8135253
> > >
> > >Testing:
> > >JPRT
> > >
> > >Thanks,
> > >Erik
> > >
> >