RFR: 8316581: Improve performance of Symbol::print_value_on() [v2]

[David Holmes]

On Wed, 20 Sep 2023 15:43:18 GMT, Thomas Schatzl <tschatzl at openjdk.org> wrote:

>> Hi all,
>> 
>>   please review this (hopefully correct) optimization of `Symbol::print_value_on()`; investigation into class unloading time distribution showed that a lot of time is spent in the `UnloadingEventLog::log()` call (25+%, see CR).
>> 
>> The reason seems to be the use of `outputStream::print()` without any need for formatting.
>> 
>> This seems to decrease time spent in this logging by almost 10x.
>> 
>> Testing: hs_err output seems still be the same, GHA
>> 
>> Thanks,
>>   Thomas
>
> Thomas Schatzl has updated the pull request incrementally with one additional commit since the last revision:
> 
>   coleen review

This seems okay on the surface but raises some questions for me. Why do we have `print_value_on` and `print_symbol_on`? I get the sense that the former is somehow lower-level and potentially unsafe - which suggests that UL should not be using it in general! I could not find the original review thread for when `print_value_on` was added to answer this question, nor answer why the loop was used.

Thanks.

src/hotspot/share/oops/symbol.cpp line 393:

> 391: void Symbol::print_value_on(outputStream* st) const {
> 392:   st->print_raw("'", 1);
> 393:   static_assert(sizeof(u1) == sizeof(char), "must be");

Given the whole class assumes u1 and char equivalence this assertion seems out of place here.

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

Marked as reviewed by dholmes (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/15838#pullrequestreview-1636799185
PR Review Comment: https://git.openjdk.org/jdk/pull/15838#discussion_r1332383153