RFR(S): 7119908: G1: Cache CSet start region for each worker for subsequent reuse
bengt.rutisson at oracle.com
Thu Dec 15 08:13:37 UTC 2011
Looks good to me.
One minor thing. I think the two asserts on lines 2710 and 2711 are
unnecessary. They will never fail.
2709 HeapRegion* G1CollectedHeap::start_cset_region_for_worker(int
2710 assert(_worker_cset_start_region != NULL, "sanity");
2711 assert(_worker_cset_start_region_time_stamp != NULL, "sanity");
In the constructor for G1CollectedHeap you set the arrays up using
NEW_C_HEAP_ARRAY. If that fails to allocate memory it will call
vm_exit_out_of_memory(). So, you should never get to
start_cset_region_for_worker() with the arrays being NULL.
On 2011-12-14 23:23, John Cuthbertson wrote:
> Hi Everyone,
> Based upon some further comments from Tony, I have another new webrev
> for this change. It can be found at:
> Changes in this version include:
> * some minor name changes
> * the code that calculates the starting region was using
> total_workers() when it should have been using active_workers()
> * minor tweaks to the start and end index values to get a slightly
> better distribution.
> GC test suite
> On 12/13/11 10:39, John Cuthbertson wrote:
>> Hi Everyone,
>> I have a new webrev based upon comments from Tony, which can be found
>> at: http://cr.openjdk.java.net/~johnc/7119908/webrev.1/
>> Changes in this version include:
>> * Using the value of the existing G1CollectedHeap::_gc_time_stamp
>> field, rather than the value of _total_collections, as the TS values.
>> * A small optimization that may reduce the number of iterations while
>> walking the collection set to calculate the starting region for a
>> worker. We check the entry for the previous worker and if that is
>> valid then we starting iterating over the collection set from that
>> GC test suite; jprt.
>> On 12/09/11 11:18, John Cuthbertson wrote:
>>> Hi Everyone,
>>> Can I have a couple of volunteers to review the changes for this CR?
>>> The webrev can be found at:
>>> As part of the code review comments for 7112743 (G1: Reduce overhead
>>> of marking closure during evacuation pauses) it was suggested that
>>> instead of recalculating the starting heap region for each worker
>>> thread, we reuse the values calculated during RSet scanning. These
>>> changes address that review comment. In these changes I maintain a
>>> cache that is used and updated by
>>> G1CollectedHeap::start_cset_region_for_worker(). The first time this
>>> routine is called by a worker thread during an evacuation pause
>>> (currently during RSet scanning) the cached value for the worker
>>> gets set; when the routine is called subsequently the region that
>>> was cached for the worker is returned. I employ a simple stamp
>>> mechanism based upon the number of GCs that ensures the validity of
>>> the regions in the cache and makes clearing the cache unnecessary.
>>> GC Test suite with a small marking threshold (10%) with and without
>>> verification; the configuration of SPECjbb used to test 7117423;
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the hotspot-gc-dev