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

John Coomes John.Coomes at sun.com
Thu Mar 12 19:51:30 UTC 2009


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

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.

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) {
        ...
      }
    }

g1CollectedHeap.hpp:

229   // It specifies whether we will keep the last half-full region at

Change "It" to "This" ?

-John




More information about the hotspot-gc-dev mailing list