RFR: 8315149: Add hsperf counters for CPU time of internal GC threads [v48]

Man Cao manc at openjdk.org
Thu Nov 30 21:47:21 UTC 2023


On Thu, 30 Nov 2023 21:17:14 GMT, Jonathan Joo <jjoo at openjdk.org> wrote:

>> Me and Albert just spoke and we do see the problem that two concurrent threads could be executing the closure at the same time. So if having a total counter we need to sync the updates. But when talking we started to question how useful it is to have the gc_total counter. It is just an aggregate of the other gc-counters, but it is out of sync between safepoints. So you will always get a more accurate value by looking at the individual gc-counters.
>> 
>> We came to the conclusion that it would probably be easier to drop `gc_total` right now, rather than trying to keep it in sync for all updates to the individual counters. Because having them out of sync doesn't feel like a great option. 
>> 
>> Are we missing anything or do you agree?
>
> @simonis was the original suggester of this counter, so I will defer to his expertise. I do agree that dropping the counter would simplify things, but it also might not hurt to just leave it in. I'm okay with either option!

Right, see @simonis 's comments at https://github.com/openjdk/jdk/pull/15082#pullrequestreview-1613868256, https://github.com/openjdk/jdk/pull/15082#discussion_r1321703912.

I initially had similar thought that `gc_total` isn't necessary and provides redundant data. Now I agree with @simonis that the `gc_total` is valuable to users. It saves users from extra work of aggregating different sets of counters for different garbage collectors, and potential mistakes of missing some counters. It is also future-proof that if GC implementation changes that add additional threads, users wouldn't need to change their code to add the counter for additional threads.

I think the maintenance overhead is quite small for `gc_total` since it is mostly in this class. The benefit to users is worth it.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/15082#discussion_r1411293819


More information about the serviceability-dev mailing list