On Thu, 30 Nov 2023 09:41:55 GMT, Stefan Johansson <sjoha...@openjdk.org> wrote:
>> Jonathan Joo has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Add missing include > > 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. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/15082#discussion_r1411317828
