RFR: 8364103: Convert existing sprintf-chains to stringStream [v3]
David Holmes
dholmes at openjdk.org
Mon Sep 8 05:02:16 UTC 2025
On Fri, 5 Sep 2025 14:03:55 GMT, Paul Hübner <phubner at openjdk.org> wrote:
>> Hi all,
>>
>> This PR refactors `javaClasses`' `print_stack_element_to_stream` to use a `stringStream` instead of manually maintaining a buffer with `os::snprintf` chains.
>>
>> The JBS issue outlines to do this for all occurrences of `snprintf`-like chains. The majority of the calls to `os::snprintf`, `os::snprintf_checked`, `os::vsnprintf`, etc. occur just once. `LogTagSet::vwrite` is the only place to my knowledge where there remains a `vsnprintf` chain using manual buffer arithmetic. After consulting @jdksjolen, we decided that due to the
>> a) low-level nature of the code, and
>> b) widespread usage of logging in the VM,
>> this should probably warrant a larger discussion and/or separate issue. Personally, I feel like the performance & correctness risks of introducing a high-level abstraction here outweighs the benefits.
>>
>> I've run tests with JDK tiers 1-3 on macOS (AArch64, x64), Linux (AArch64, x64), and Windows (x64); all green.
>
> Paul Hübner has updated the pull request incrementally with one additional commit since the last revision:
>
> Clean up method a bit.
Changes look good. One suggestion around the lengthy new comment.
Thanks
src/hotspot/share/classfile/javaClasses.cpp line 2590:
> 2588: // Presumably to avoid interleaving with other threads.
> 2589: // The current version retains that spirit by writing to a stringStream first before
> 2590: // flushing everything into the outputStream at once.
Suggestion:
// The output is formatted into a stringStream and written to the outputStream in one step.
No need to comment on history as such.
-------------
Marked as reviewed by dholmes (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/26894#pullrequestreview-3194927167
PR Review Comment: https://git.openjdk.org/jdk/pull/26894#discussion_r2329107480
More information about the hotspot-runtime-dev
mailing list