On Thu, 4 May 2023 19:54:57 GMT, Paul Hohensee <p...@openjdk.org> wrote:
> Please review this addition to com.sun.management.ThreadMXBean that returns > the total number of bytes allocated on the Java heap since JVM launch by both > terminated and live threads. > > Because this PR adds a new interface method, I've updated the JMM_VERSION to > 4, but would be happy to update it to 3_1 instead. Looks good in general. Please find my comments inline. src/hotspot/share/include/jmm.h line 55: > 53: JMM_VERSION_2 = 0x20020000, // JDK 10 > 54: JMM_VERSION_3 = 0x20030000, // JDK 14 > 55: JMM_VERSION_3_0 = 0x20030000, Why do we need `JMM_VERSION_3_0`? We haven't defined `JMM_VERSION_2_0` either. src/hotspot/share/include/jmm.h line 321: > 319: jstring flag_name, > 320: jvalue new_value); > 321: jlong (JNICALL *GetAllThreadAllocatedMemory) I'm not sure here, but I think there's no need to "overwrite" a *reserved* slot if you add this functionality to a new major release as you do. You also haven't done it when you've added `GetOneThreadAllocatedMemory()` with [JDK-8231209](https://bugs.openjdk.org/browse/JDK-8231209). I think we should keep these *reserved* slots for the case when we eventually have to downport new functionality from a later release. src/hotspot/share/services/management.cpp line 2282: > 2280: jmm_FindDeadlockedThreads, > 2281: jmm_SetVMGlobal, > 2282: jmm_GetAllThreadAllocatedMemory, See comment on overwriting the `reserved6` slot above. ------------- PR Review: https://git.openjdk.org/jdk/pull/13814#pullrequestreview-1414984240 PR Review Comment: https://git.openjdk.org/jdk/pull/13814#discussion_r1186213343 PR Review Comment: https://git.openjdk.org/jdk/pull/13814#discussion_r1186224124 PR Review Comment: https://git.openjdk.org/jdk/pull/13814#discussion_r1186301611
