RFR (M): JDK-8040722: G1: Clean up usages of heap_region_containing

[Thomas Schatzl]

Hi Bengt,

On Thu, 2014-04-17 at 10:26 +0200, Bengt Rutisson wrote:
> Hi again,
> 
> Here's an updated webrev:
> http://cr.openjdk.java.net/~brutisso/8040722/webrev.01/
> 
> And here's the incremental change compared to the previous one:
> http://cr.openjdk.java.net/~brutisso/8040722/webrev.00-01.diff/
> 

  a few comments, none of them are critical:

 - g1CollectedHeap.inline.hpp, in
G1CollectedHeap::heap_region_containing_raw:

The != NULL check is subsumed by the other. Is it possible to merge them
with a useful error message?

E.g. assert(_g1_reserved.contains(...), err_msg("Address "PTR_FORMAT" is
outside of the heap ranging from ["PTR_FORMAT" to "PTR_FORMAT")", addr,
_g1_reserved.start(), _g1_reserved.end());

I would prefer to have one assert instead of separating them.

 - in CMTask::setup_for_region(): maybe the error message for the !=
NULL assert could be improved.

 - G1CollectedHeap::heap_region_containing_raw documentation: maybe
better:

// Returns the HeapRegion the given address addr contains. Addr must not
be NULL.

 - G1CollectedHeap::heap_region_containing() documentation: I would
prefer something like:

// Returns the HeapRegion the given address addr contains. Addr must not
be NULL. If addr is within a humongous continues region, it returns its
humongous start region.

Imo, to name a region, using "<type-name> region" (e.g. humongous
continues region) is better than trying to invent new descriptions like
continuing humonguous region.

Thanks,
Thomas