RFR: JDK-8266503: [UL] Make Decorations safely copy-able and reduce their size [v2]

[Xin Liu]

On Thu, 6 May 2021 12:00:21 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> This patch reduces the size of UL `LogDecorations` by about 85% (from 368 -> 56 bytes on Linux x64). This matters in the context of asynchronous logging in UL where we plan to keep a buffer containing log messages, including decorations, for asynchronous printing.
>> 
>> As a side effect, it makes the LogDecorations object safe to copy with trivial assignment constructors and operators (which it had not been before).
>> 
>> As another side effect, the 256-char-for-all-decorators limit has been removed with this patch.
>> 
>> What the patch does:
>> 
>> In LogDecorations, we resolve the values of the given decorators ("uptime", "tid" etc) and print them in human-readable format. Before this patch, the class LogDecorations stored the printed decorators in an internal (limited, fixed-sized) buffer. This is inefficient since this takes much more memory than storing the binary data before printing them.
>> 
>> So this patch separates the decorator value resolving from the printing. It stores the resolved values in binary form and only prints them when needed. Since a decorations object is only printed once this is fine. No need to cache the formatted text.
>> 
>> *Please Note that this patch includes the fix for JDK-8266536: "Provide a variant of os::iso8601_time which works with arbitrary timestamps" which is in a separate PR (https://github.com/openjdk/jdk/pull/3869) - so please ignore all the iso6801 stuff. It also contains a fix for the broken harfbuzz build.*
>> 
>> Testing:
>> - gtests, manually
>> - jtreg runtime/logging, manually
>> - SAP nightlies ran fine, but since then the tests changed, will run them again tonight
>
> Thomas Stuefe has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains six additional commits since the last revision:
> 
>  - Merge
>  - Remove harfbuzz build fix
>  - Volker feedback
>  - start
>  - cherry-picked JDK-8266536
>  - harfbuzz-buildfix

src/hotspot/share/logging/logDecorations.cpp line 74:

> 72: }
> 73: 
> 74: void LogDecorations::print_decoration(LogDecorators::Decorator decorator, outputStream* st) const {

Is that a good idea to do a sanity check here in debug build? 

In my understanding, calling `print_#name##decoration()` for a decorator which was not set in ctor's decorators is undefined.  Currently, your code is safe because you carefully initialize each member variable in initializer list. You can't assume follower programmers do the same. 

If you save the variable `decorators`(a bitmask), you can do sanity check here.  This can be done in debug build. eg. 
`assert(_decorators.is_decorator(decorator), "sanity check");`

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

PR: https://git.openjdk.java.net/jdk/pull/3874