On Thu, 30 Nov 2023 02:39:39 GMT, Jonathan Joo <j...@openjdk.org> wrote:
>> This two-step update does seem unnecessary, IMO. > > I agree that in the case that multiple closures are not active at the same > time, we wouldn't have to implement it in this way. However, isn't it > possible to have multiple closures active simultaneously, e.g. vm thread, > concurrent mark thread, concurrent refine thread? I think to account for the > races there, we can only update the `_gc_total_cpu_time_diff` member variable > atomically during these closure destructions, and then publish the actual > updated `gc_total` counter at manually specified times via > `publish_gc_total_cpu_time()`. If we were to call `publish_gc_total_cpu_time` > as part of the thread closure, I believe it would be difficult to prevent > races when accessing the underlying counter from the various gc-related > threads. > > Or maybe there is another strategy that I'm not seeing? Both `publish_gc_total_cpu_time` and `~ThreadTotalCPUTimeClosure` are called by the vm-thread inside a safepoint, so there shouldn't be any other threads running simultaneously, I believe. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/15082#discussion_r1410221646
