On Mon, 19 Aug 2024 15:39:02 GMT, Stefan Karlsson <stef...@openjdk.org> wrote:
> The `ClassLoadingMXBean` and `MemoryMXBean` APIs have `setVerbose` methods to > control verbose mode and `isVerbose` methods to query it. Some JCK tests > expect `setVerbose(false)` to disable verbose mode and, subsequently, > `isVerbose()` to return false. However, if logging to a file is enabled by > using -Xlog on the java launcher command line then `isVerbose()` returns true > even after calling `setVerbose(false)`. > > The proposed patch solves this by introducing two changes: > > 1) The previous implementation used the `log_is_enabled` functionality to > check if logging was enabled for the given tag set. This returns true if > logging has been turned on for *any* output. The patch changes this so that > `isVerbose` only checks what has been configured for stdout, which is the > output that `setVerbose` configures. > > 2) The previous implementation of `setVerbose` turned on `class+load*` > (notice the star) but then `isVerbose` only checked `class+load` (without the > star). The patch changes this so that the `isVerbose` in-effect checks > `class+load*`. (The `gc` part of the patch did not have this problem) > > The main focus on this patch is to fix the JCK failure, with an > implementation that follows the API documentation. While looking at this area > of the code it is clear that there are other problems that we might want to > addressed in the future, but we're intentionally keeping this patch limited > in scope so that it can be backported to JDK 23. > > A CSR for this change has been created. > > Testing: > * The newly implemented tests > * The failing JCK tests with the corresponding -Xlog lines > * Tier1-7 (running) > > The patch is co-authored by me and David Holmes Thumbs up. src/hotspot/share/services/memoryService.cpp line 218: > 216: > 217: return false; > 218: } If we have `-Xlog:gc` and `-Xlog:gc+foo` set I think this version of `MemoryService::get_verbose()` will return `false`. Is that really what we want? test/jdk/java/lang/management/ClassLoadingMXBean/TestVerboseClassLoading.java line 36: > 34: * @run main/othervm -Xlog:class+load=trace TestVerboseClassLoading false > 35: * @run main/othervm -Xlog:class+load=debug TestVerboseClassLoading false > 36: * @run main/othervm -Xlog:class+load=info TestVerboseClassLoading false Hmm... I was expecting these to be `true`. What am I missing? test/jdk/java/lang/management/ClassLoadingMXBean/TestVerboseClassLoading.java line 56: > 54: */ > 55: > 56: import java.lang.management.*; I thought we tried to avoid wild-card imports. test/jdk/java/lang/management/MemoryMXBean/TestVerboseMemory.java line 50: > 48: */ > 49: > 50: import java.lang.management.*; I thought we tried to avoid wild-card imports. ------------- Marked as reviewed by dcubed (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/20628#pullrequestreview-2245990507 PR Review Comment: https://git.openjdk.org/jdk/pull/20628#discussion_r1722060516 PR Review Comment: https://git.openjdk.org/jdk/pull/20628#discussion_r1722064641 PR Review Comment: https://git.openjdk.org/jdk/pull/20628#discussion_r1722072308 PR Review Comment: https://git.openjdk.org/jdk/pull/20628#discussion_r1722072710
