Request for review (s) : 8017462: G1: guarantee fails with UseDynamicNumberOfGCThreads
jon.masamitsu at oracle.com
Tue Mar 17 17:33:22 UTC 2015
On 03/10/2015 11:45 AM, Thomas Schatzl wrote:
> Hi Jon,
> On Tue, 2015-03-10 at 09:26 -0700, 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.
> - I would somewhat prefer if _active_threads would not be stored in
> every WorkerDataArray. This seems a waste of space. G1GCPhaseTimes
> already stores it, so it could easily passed to
> WorkerDataArray::verify()/print() instead which seems cleaner to me.
Having seen Bengt's patch for this, I agree and will
abandon my fix.
> This would also avoid the additional parameter in
> - WorkerDataArray::reset() is PRODUCT_RETURN. That means that in a
> product VM, WorkerDataArray::_active_length is seemingly never assigned
> to then as far as I can see.
> - is it possible to create a test for G1 that runs with
> G1LogLevel=finest and parses and verifies output of one GC phase that
> takes at least some time? I.e. that you have active_workers amount of
> durations in that line and the sum matches more or less.
Let me look into this. Parsing the output is worrisome to
me. I'll look to see if I can add assertion check to check
> - Bengt is currently also changing this code significantly (see the
> review for 8074037: Refactor the G1GCPhaseTime logging to make it easier
> to add new phases). Somebody will have to do a significant amount of
> merging :/
More information about the hotspot-gc-dev