RFR: 8338139: {ClassLoading, Memory}MXBean::isVerbose methods are inconsistent with their setVerbose methods

[Daniel D. Daugherty]

On Mon, 19 Aug 2024 15:39:02 GMT, Stefan Karlsson <stefank at 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