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

[Volker Simonis]

On Fri, 7 May 2021 10:21:10 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 pull request now contains ten commits:
> 
>  - Merge
>  - Xins debug check proposal
>  - LogTagSet::describe_tagsets can use the stream interface of label and avoid copying
>  - Merge
>  - Remove harfbuzz build fix
>  - Volker feedback
>  - start
>  - cherry-picked JDK-8266536
>  - harfbuzz-buildfix

Hi Thomas,

everything looks fine now except the comments below (i.e. didn't you wanted to use the new `iso8601_timestamp_size`?).

If that 's clarified I'll approve this PR.

Thanks,
Volker

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

> 92: 
> 93: void LogDecorations::print_time_decoration(outputStream* st) const {
> 94:   char buf[29];

Didn't you wanted to use the new `iso8601_timestamp_size` constant here?

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

> 98: 
> 99: void LogDecorations::print_utctime_decoration(outputStream* st) const {
> 100:   char buf[29];

Didn't you wanted to use the new `iso8601_timestamp_size` constant here?

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

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