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