Code review request: 6604422: G1: re-use half-promoted regions (S)

[Tony Printezis]

John,

John Coomes wrote:
> Tony Printezis (Antonios.Printezis at Sun.COM) wrote:
>   
>> Ensures that the last region allocated to during one GC is re-used, when 
>> possible, during the next GC and it's not left half-full.
>>
>> http://cr.openjdk.java.net/~tonyp/6604422/webrev.0/
>>     
>
> I'm not a g1 expert, but generally looks good.  I found a few trivial
> things.
>
> g1CollectedHeap.cpp:
>
> 2882       // let's make sure that the GC alloc regions is not tagged as such
>   
Done.
> Change "regions" to "region".
>
> 2893         // note that _gc_alloc_region[ap] will be nulled below, before
> 2894         // a new one is obtained
>
> Is the above correct?  I don't see it explicitly nulled.
>   
No. :-) In fact there's an assert further up that says that 
_gc_alloc_regions[ap] should already be NULL. I changed the code and 
forgot to remove the comment. Thanks.
> In G1CollectedHeap::release_gc_alloc_regions(bool totally), I think it
> would be more readable if the 3 separate tests for (r != NULL) were
> combined into one.  E.g.,
>
>     if (r != NULL) {
>       // we retain nothing on _gc_alloc_regions between GCs
>       set_gc_alloc_region(ap, NULL);
>       _gc_alloc_region_counts[ap] = 0;
>
>       if (r->is_empty()) {
>         ...
>       } else if (_retain_gc_alloc_region[ap] && !totally) {
>         ...
>       }
>     }
>   
Yeah, good point. I changed it (and added a couple of extra comments 
too). Again, I changed the code a few times and didn't restructure it at 
the end.
> g1CollectedHeap.hpp:
>
> 229   // It specifies whether we will keep the last half-full region at
>
> Change "It" to "This" ?
>   
Done, thanks! Andrey suggested to piggyback a tiny fix from another CR 
on this. I'll do some testing on the new version and publish the new 
webrev soon-ish.

Tony

-- 
----------------------------------------------------------------------
| Tony Printezis, Staff Engineer    | Sun Microsystems Inc.          |
|                                   | MS BUR02-311                   |
| e-mail: tony.printezis at sun.com    | 35 Network Drive               |
| office: +1 781 442 0998 (x20998)  | Burlington, MA01803-0902, USA  |
----------------------------------------------------------------------
e-mail client: Thunderbird (Solaris)