Request for review (s) : 8017462: G1: guarantee fails with UseDynamicNumberOfGCThreads

Derek White derek.white at
Tue Mar 10 16:43:55 UTC 2015

On 3/10/15 12:26 PM, Jon Masamitsu wrote:
> 8017462: G1: guarantee fails with UseDynamicNumberOfGCThreads
> When fewer than the maximum number of threads was being used for
> a parallel section, phase times for the threads that did not execute and
> averages for the phase were misleading.  The fix passes the active 
> number of
> GC threads  to the G1 phase timers.
> Tested with gc_test_suite.
Hi Jon,

Looks good to me.

Just a question: In g1StringDedup.cpp:

  void G1StringDedup::unlink_or_oops_do(BoolObjectClosure* is_alive, OopClosure* keep_alive, bool allow_resize_and_rehash) {
    assert(is_enabled(), "String deduplication not enabled");
-  G1CollectorPolicy* g1p = G1CollectedHeap::heap()->g1_policy();
-  g1p->phase_times()->note_string_dedup_fixup_start();
+  G1CollectedHeap* g1h = G1CollectedHeap::heap();
+  G1CollectorPolicy* g1p = g1h->g1_policy();
+  uint active_workers = g1h->workers()->active_workers();
+  g1p->phase_times()->note_string_dedup_fixup_start(active_workers);
    double fixup_start = os::elapsedTime();
    G1StringDedupUnlinkOrOopsDoTask task(is_alive, keep_alive, allow_resize_and_rehash);
-  G1CollectedHeap* g1h = G1CollectedHeap::heap();
-  g1h->set_par_threads();
+  g1h->set_par_threads(active_workers);

You switched from not changing the number of active workers 
"set_par_threads()" to explicitly setting the
number "g1h->set_par_threads(active_workers)".

Do you expect to get a different number of active threads this way, or 
are you just making the code clearer by
making the count explicit?

  - Derek
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <>

More information about the hotspot-gc-dev mailing list