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

[Axel Boldt-Christmas]

On Wed, 9 Oct 2024 09:13:37 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:
> 
>   Replace LogDecorators::Decorator::NoDecorator with LogDecorators::None

Seems like on of my comments (the large one) got lost. Trying this again. :)

src/hotspot/share/logging/logDecorators.hpp line 96:

> 94: 
> 95:     const LogSelection& selection() const { return _selection; }
> 96:   };

I am uncomfortable with this type erasure. `LogTagType[LogTag::MaxTags + 1 /* = 6 */]` -> `LogTagType*` -> `LogTagType[LogTag::MaxTags /* = 5 */]`. I think this should be rewritten so that `tag_arr` is typed as a `LogTagType[5]`. I think everywhere we have a `const LogTagType parameter[LogTag::MaxTags]` really should have been `const LogTagType (&parameter)[LogTag::MaxTags]` so that this would have been a compile error. 

My suggestion is to either do the following:
Suggestion:

  public:
    DefaultUndecoratedSelection(LogLevelType level, LogTagType t0, LogTagType t1 = LogTag::__NO_TAG,
                                LogTagType t2 = LogTag::__NO_TAG, LogTagType t3 = LogTag::__NO_TAG,
                                LogTagType t4 = LogTag::__NO_TAG, LogTagType guard_tag = LogTag::__NO_TAG) : _selection(LogSelection::Invalid) {
      assert(guard_tag == LogTag::__NO_TAG, "Too many tags specified!");

      LogTagType tag_arr[LogTag::MaxTags] = { t0, t1, t2, t3, t4 };

      _selection = LogSelection(tag_arr, false, level);
    }

    const LogSelection& selection() const { return _selection; }
  };


or maybe even better, do what we do for the `LogTagSet` and have a static helper and a private constructor, so that we can turn all the asserts into compile errors. 

Something like:
Suggestion:

    DefaultUndecoratedSelection(LogLevelType level, LogTagType t0, LogTagType t1, LogTagType t2,
                                LogTagType t3, LogTagType t4) : _selection(LogSelection::Invalid) {
      LogTagType tag_arr[LogTag::MaxTags] = { t0, t1, t2, t3, t4 };
      _selection = LogSelection(tag_arr, false, level);
    }
  public:

    template <LogLevelType Level, LogTagType T0, LogTagType T1 = LogTag::__NO_TAG, LogTagType T2 = LogTag::__NO_TAG,
              LogTagType T3 = LogTag::__NO_TAG, LogTagType T4 = LogTag::__NO_TAG,
              LogTagType GuardTag = LogTag::__NO_TAG>
    static DefaultUndecoratedSelection make() {
      STATIC_ASSERT(GuardTag == LogTag::__NO_TAG);
      return DefaultUndecoratedSelection(Level, T0, T1, T2, T3, T4);
    }

    const LogSelection& selection() const { return _selection; }
  };


And we can then use `LogDecorators::DefaultUndecoratedSelection::make<level, LOG_TAGS(...)>()` to create them.

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

Changes requested by aboldtch (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/21383#pullrequestreview-2367260954
PR Review Comment: https://git.openjdk.org/jdk/pull/21383#discussion_r1799872623