RFR (M): 8071913: Filter out entries to free/uncommitted regions during iteration

[Kim Barrett]

> On Oct 2, 2018, at 7:28 AM, Thomas Schatzl <thomas.schatzl at oracle.com> wrote:
> 
> […]
> 
> CR:
> https://bugs.openjdk.java.net/browse/JDK-8071913
> Webrev:
> http://cr.openjdk.java.net/~tschatzl/8071913/webrev/
> Testing:
> hs-tier1-5,jdk-tier1-3, perf tested, no change in pause times or
> (throughput) scores
> 
> Quite a few gc tests do extensive commit/uncommit of regions with the
> corresponding exercise of the code.
> 
> Thanks,
>  Thomas

Looks good.

A few minor comments, for which I don't need a new webrev.

------------------------------------------------------------------------------
src/hotspot/share/gc/g1/g1CollectedHeap.hpp
1123   inline HeapRegion* region_at_or_null(uint index) const;

Needs comment updating.  Presumably this too requires the index to be
valid, and only returns NULL if the region is unmapped.

------------------------------------------------------------------------------
src/hotspot/share/gc/g1/g1CollectedHeap.inline.hpp
  95   uint const region_idx = addr_to_region(addr);
  96   return _hrm.is_available(region_idx) ? region_at(region_idx) : NULL;

Shouldn't this just be

  _hrm.at_or_null(addr_to_region(addr));

------------------------------------------------------------------------------
src/hotspot/share/gc/g1/g1CollectedHeap.cpp
2672           if (!g1h->is_in_closed_subset(ct->addr_for(card_ptr))) {
2673             continue;
2674           }

I prefer the original code, without this change.

------------------------------------------------------------------------------
src/hotspot/share/gc/g1/g1CollectedHeap.cpp
[deleted code]
2679         assert(hrrs.n_yielded() == r->rem_set()->occupied(),
2680                "Remembered set hash maps out of sync, cur: " SIZE_FORMAT " entries, next: " SIZE_FORMAT " entries",
2681                hrrs.n_yielded(), r->rem_set()->occupied());

I'm not sure how this code deletion relates to the other changes in
this webrev.

------------------------------------------------------------------------------ 
src/hotspot/share/gc/g1/heapRegionManager.hpp
 126 public:

Moving that makes for easier reviewing, which is good.  But I think it
might be better to move the declaration of is_available to some better
part of the public functions, maybe near at_or_null?

------------------------------------------------------------------------------
src/hotspot/share/gc/g1/g1RemSet.hpp
src/hotspot/share/gc/g1/heapRegion.hpp
src/hotspot/share/gc/g1/sparsePRT.hpp
src/hotspot/share/gc/g1/sparsePRT.cpp
[no changes]

------------------------------------------------------------------------------