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

Reply via email to