On Thu, 30 Nov 2023 21:59:41 GMT, Man Cao <m...@openjdk.org> wrote: >> src/hotspot/share/runtime/cpuTimeCounters.hpp line 59: >> >>> 57: NONCOPYABLE(CPUTimeCounters); >>> 58: >>> 59: static CPUTimeCounters* _instance; >> >> I would prefer if we made the whole class static and got rid of the instance >> and just made the `_cpu_time_counters` array static. The only drawback I/we >> (discussed this with Albert as well) can see is that the memory for the >> array would not be accounted in NMT, but this array will always be very >> small so should not be a big problem. >> >> Do you see any other concerns? > > I thought it is typically preferred to initialize a singleton object on the > heap, rather than using several static variables. It is easier to control the > initialization order and timing of an on-heap singleton object than statics. > > Moreover, for this class, `initialize()` could also check `if (UsePerfData)`, > and only create the singleton object under `UsePerfData`. This could save > some memory when `UsePerfData` is false.
I would say it depends on the use-case and here when switching to use static functions to use the instance it felt more like an all-static class. I agree that it would be nice to avoid the additional memory usage if `UsePerfData` is `false` so I'm ok with keeping the instance if we add that. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/15082#discussion_r1411872139
