On Wed, 29 Nov 2023 15:23:55 GMT, Albert Mingkun Yang <[email protected]> wrote:
>> We could add a new closure just used by GC that 's a sub-class of
>> `ThreadTotalCPUTimeClosure` and just adds this to the constructor:
>>
>> instance->inc_gc_total_cpu_time(net_cpu_time);
>>
>>
>> That way we could get rid of `CPUTimeGroups::is_gc_counter()` as well since
>> all those counters should use the "GC closure" or we can keep it and assert
>> that no GC closure uses the wrong closure.
>>
>> What do you think about that Albert, would that address your concerns?
>
> (I just realized that I made a typo in my previous msg; should be *callee*
> instead.) That is what I have in mind.
>
>
> void CPUTimeCounters::update_counter(name, total) {
> auto counter = get_counter(name);
> auto old_v = counter->get_value();
> auto diff = total - old_v;
> counter->inc(diff);
> if (counter->is_gc_counter()) {
> counter->inc(diff);
> }
> }
I'm not sure I understood correctly, could you let me know if this latest
commit addresses your comment in the way you were intending?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/15082#discussion_r1410059322
