On Tue, 28 Nov 2023 02:22:45 GMT, Jonathan Joo <j...@openjdk.org> wrote:
>> 8315149: Add hsperf counters for CPU time of internal GC threads > > Jonathan Joo has updated the pull request incrementally with two additional > commits since the last revision: > > - Fix namespace issues (2) > > Co-authored-by: Stefan Johansson > <54407259+kstef...@users.noreply.github.com> > - Fix namespace issues > > Co-authored-by: Stefan Johansson > <54407259+kstef...@users.noreply.github.com> >From a testing point of view I think this is looking good now, re-ran the >failing test and some other jstat tests as well and they all pass. Please address the comments from Albert and we can hopefully finish this before RDP1. src/hotspot/share/runtime/cpuTimeCounters.cpp line 91: > 89: } while (old_value != fetched_value); > 90: get_counter(CPUTimeGroups::CPUTimeType::gc_total)->inc(fetched_value); > 91: } Why do we have to do this publish dance? Couldn't the closure that update the diff instead just update the counter? From what I can tell we never have multiple closures active at the same time so should be no race there? ------------- PR Review: https://git.openjdk.org/jdk/pull/15082#pullrequestreview-1754686415 PR Review Comment: https://git.openjdk.org/jdk/pull/15082#discussion_r1408914724
