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

Thomas Stuefe stuefe at openjdk.java.net
Thu May 6 12:05:59 UTC 2021


On Thu, 6 May 2021 09:40:42 GMT, Volker Simonis <simonis at openjdk.org> wrote:

>> 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
>
> Hi Thomas,
> 
> your change looks good and I like it :)
> 
> Am I right that you've changed the 256-char-for-all-decorators limit to a 255-char limit for a single decorator (which is obviously much better)?
> 
> Find my other comments inline.
> 
> Thank you and best regards,
> Volker

Hi @simonis , @YaSuenag , thanks for your Reviews!

New version - mostly driven by Volkers feedback:

- I now cache the pid, which further reduces the size of this object to 48. As mentioned in https://github.com/openjdk/jdk/pull/3874#discussion_r627326425, one could further reduce it but I think this is already fine.
- I added `LogDecorations::max_decoration_size` and used it for temp buffers. Note that this limit would be easy to remove by using `fileStream` in `LogFileStreamOutput::write_decorations()` - however, `fileStream` misses some functionality and I wanted to limit the patch size.
- I fixed the error handling for `os::iso8601_time()`
- I added a overload to `LogTagSet` which takes an `outputStream` - this is more elegant and prevents the copy. I also added a gtest for this function
- I removed the harfbuzz build fix

Thanks for reviewing!

..Thomas

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

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


More information about the hotspot-runtime-dev mailing list