On Thu, 30 Nov 2023 21:17:14 GMT, Jonathan Joo <j...@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