RFR: 8135253: Add push method to CollectionSetChooser

Erik Helin erik.helin at oracle.com
Wed Sep 9 13:11:43 UTC 2015


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



More information about the hotspot-gc-dev mailing list