RFR (XXS): 8133456: HeapRegionManager::shrink_by() iterates suboptimally across regions

[Thomas Schatzl]

Hi,

On Fri, 2015-08-21 at 10:27 +0200, Bengt Rutisson wrote:
> Hi Thomas,
> 
> On 2015-08-21 10:05, Thomas Schatzl wrote:
> > Hi all,
> >
> >    can I have reviews for this one-line change that improves efficiency
> > during heap shrinking?
> >
> > The problem was that the increment to skip over already scanned regions
> > has been too conservative. In some cases it would mean that regions in
> > the region table would be scanned twice.
> >
> > CR:
> > https://bugs.openjdk.java.net/browse/JDK-8133456
> > Webrev:
> > http://cr.openjdk.java.net/~tschatzl/8133456/webrev/
> 
> The fix looks good, but do we really need both cur and idx_last_found? 
> After your fix we just set them to the same value. Technically it would 
> be enough that find_empty_from_idx_reverse() takes one parameter that is 
> both the in and out value. But it may be cleaner to have the two separated.
> 
> I'll leave it up to you if you want to change it. I'm fine with the 
> proposed patch too.
> 

  I would be fine with either way too, but I have received some review
comments in the past (recently about 8067336 where originally I had one
in and one inout parameter instead of three parameters (in/in/out) for
all methods) to avoid inout parameters since it is easier to understand.

Let's get another opinion.

Thanks,
  Thomas