RFR: 8364103: Convert existing sprintf-chains to stringStream [v2]
Kim Barrett
kbarrett at openjdk.org
Thu Sep 4 17:52:45 UTC 2025
On Thu, 4 Sep 2025 13:12:35 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 with a new target base due to a merge or a rebase. The pull request now contains three commits:
>
> - Incorporate reviewer feedback.
> - Merge branch 'master' into JDK-8364103
> - Refactor stack trace printing to use stringStream.
Looks okay as is, but there is some optional tidying up that could be done.
src/hotspot/share/classfile/javaClasses.cpp line 2615:
> 2613: }
> 2614:
> 2615: stringStream ss;
Consider moving the stringStream declaration up near the front of the function, and moving the print of
`klass_name` and `method_name` earlier, to just after they are set up. And consider moving the setup
of `source_file_name` down closer to where it's used. The early setups before printing were needed in
the old code as part of calculating the buffer length. But that's no longer a concern.
src/hotspot/share/classfile/javaClasses.cpp line 2648:
> 2646: nmethod* nm = method->code();
> 2647: if (WizardMode && nm != nullptr) {
> 2648: ss.print("(nmethod " INTPTR_FORMAT ")", (intptr_t)nm);
pre-existing: Usual practice is to use `p2i(nm)` rather than a cast.
-------------
Marked as reviewed by kbarrett (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/26894#pullrequestreview-3186410195
PR Review Comment: https://git.openjdk.org/jdk/pull/26894#discussion_r2322968092
PR Review Comment: https://git.openjdk.org/jdk/pull/26894#discussion_r2322951743
More information about the hotspot-runtime-dev
mailing list