RFR: 8341622: Tag-specific disabled default decorators for UnifiedLogging [v4]

[Roberto Castañeda Lozano]

On Mon, 7 Oct 2024 13:26:56 GMT, Antón Seoane <duke at openjdk.org> wrote:

>> Currently, the Unified Logging framework defaults to three decorators (uptime, level, tags) whenever the user does not specify otherwise through -Xlog. However, some specific logging cases do not need decorations, and manually having to disable them results in cumbersome extra input and loss of ergonomics. For example, C2 developers rarely need decorations, and having to manually specify this every time results inconvenient.
>> 
>> To address this, this PR enables the possibility of adding tag-specific disabling of default decorators to UL. These disables are in no way overriding user input -- they will only act whenever -Xlog has no decorators supplied and there is a positive match with the pre-specified defaults. Such a match is based on an inclusion rule: e.g. if -Xlog:jit+compilation is provided, a default for jit may be applied. Additionally, defaults may target a specific log level.
>> 
>> The original use case for this is related to C2 logging migration to UnifiedLogging, as currently no decorators are found in compiler logs and it would be expected to stay the same without the extra explicit removal every time via -Xlog. However, this would ease the migration of other logging that was initially deterred by this, such as -XX:+PrintInterpreter.
>> 
>> This PR is a simplification of the [8340363](https://bugs.openjdk.org/browse/JDK-8340363) (closed) ticket.
>
> Antón Seoane has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Update full name

Thanks for working on this, Antón! The new test files look good to me, I also agree on hiding decorators by default for `jit+inlining`. I just have a few minor comments.

test/hotspot/gtest/logging/test_logDefaultDecorators.cpp line 29:

> 27: #include "logging/logDecorators.hpp"
> 28: #include "runtime/os.hpp"
> 29: #include "unittest.hpp"

Please sort the included files alphabetically (except for `precompiled.hpp` which should go first) for consistency with the other test files in the directory. Also, `runtime/os.hpp` is unused.

Suggestion:

#include "precompiled.hpp"
#include "jvm.h"
#include "logging/logDecorators.hpp"
#include "logging/logTag.hpp"
#include "unittest.hpp"

test/hotspot/jtreg/runtime/logging/DefaultLogDecoratorsTest.java line 27:

> 25:  * @test
> 26:  * @requires vm.flagless
> 27:  * @summary Running -Xlog with tags which have default decorators should pick them

This summary reflects the old proposal in JDK-8340363, please update.

test/hotspot/jtreg/runtime/logging/DefaultLogDecoratorsTest.java line 51:

> 49:         for (String string : xlog) {
> 50:             argsList.add(string);
> 51:         }

Suggestion:

        List<String> argsList = new ArrayList(Arrays.asList(xlog));

test/hotspot/jtreg/runtime/logging/DefaultLogDecoratorsTest.java line 69:

> 67:         doTest(false, "-Xlog:jit+inlining*=trace:decorators.log");
> 68: 
> 69: 

Nit: unnecessary extra line (same for the other lines below).

-------------

Changes requested by rcastanedalo (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/21383#pullrequestreview-2353875217
PR Review Comment: https://git.openjdk.org/jdk/pull/21383#discussion_r1791493896
PR Review Comment: https://git.openjdk.org/jdk/pull/21383#discussion_r1791501653
PR Review Comment: https://git.openjdk.org/jdk/pull/21383#discussion_r1791672411
PR Review Comment: https://git.openjdk.org/jdk/pull/21383#discussion_r1791512229