On Wed, 1 Nov 2023 09:34:01 GMT, Stefan Johansson <sjoha...@openjdk.org> wrote:
>> Jonathan Joo has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Replace NULL with nullptr > > Sorry for being a bit late to this PR. I think the addition of CPU time > tracking is good, but I wonder if we could do it in a way that is a bit more > general. A more general way of tracking CPU time for a set of threads and we > could then have different consumers of this data. In addition to hsperf > counters I think having logging and JFR events for this could be interesting > as well. > > Have you had any thought along those lines, any obvious problems with such > approach? @kstefanj thank you for taking a look! caoman@ and I discussed your comment and our thoughts are below: > A more general way of tracking CPU time for a set of threads and we could > then have different consumers of this data. Could you elaborate a bit on what you were thinking of here? If we are assuming something like a thread that updates all other threads, I think this implementation could get a bit complicated. There are two main issues that we can see with a generic thread approach: 1. We would have to figure out how often to pull metrics from the various gc threads from the central thread, and possibly determine this frequency separately for every thread. Instead with our current implementation, we can manually trigger publishes based on when the GC thread is done doing work. 2. We would still need to tag each thread we want to track somewhere, and keep track of a mapping from thread to its counter name, etc. which doesn't seem to simplify things too much. (I imagine we will still need to touch numerous files to "tag" each thread with whether we want to track it or not?) The existing `sun.management:type=HotspotThreading` MBean approach (discussed [here]( https://mail.openjdk.org/pipermail/core-libs-dev/2023-September/111397.html)) could be another general way to track CPU. However, the discussion concludes that it is an internal API, and discourages users from using it. > In addition to hsperf counters I think having logging and JFR events for this > could be interesting as well. There's actually already an existing implementation that covers process CPU from GC pauses, but it doesn't handle concurrent work: https://bugs.openjdk.org/browse/JDK-8291753. However, for our implementation of AHS, we decided against a JFR-based approach, as it has a bit more overhead and the hsperf-based implementation seemed simpler. So while a JFR-based approach to track these counters might be feasible, we believe it would be considerably more work, and could be done in a separate PR later if there is sufficient user interest. ------------- PR Comment: https://git.openjdk.org/jdk/pull/15082#issuecomment-1789914128
