RFR: 8364103: Convert existing sprintf-chains to stringStream

Ioi Lam iklam at openjdk.org
Fri Aug 22 18:26:53 UTC 2025


On Fri, 22 Aug 2025 08:49:56 GMT, Paul Hübner <duke 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.

Why are we printing to a buffer first?

Originally this function was `java_lang_Throwable::print_stack_element_to_buffer()` and returns a buffer. Hence the anachronistic comment "// Print stack trace element to resource allocated buffer"  which is no longer correct.

I can't find any information from the history of this file:

https://github.com/openjdk/jdk/blame/68f3dd76c92ce3b0219ac2cc346718dd865645d9/hotspot/src/share/vm/classfile/javaClasses.cpp#L1161

I am guessing the reason is to avoid interleaving with other threads. I think the above comment should be updated to reflect this.

src/hotspot/share/classfile/javaClasses.cpp line 2591:

> 2589:   ResourceMark rm;
> 2590: 
> 2591:   // Get strings.

This comment is useless and should be removed.

src/hotspot/share/classfile/javaClasses.cpp line 2613:

> 2611: 
> 2612:   // The string stream that will handle all of the formatting and outputting.
> 2613:   stringStream ss;

This comment is not necessary.

src/hotspot/share/classfile/javaClasses.cpp line 2616:

> 2614: 
> 2615:   // Print stack trace line in buffer
> 2616:   ss.print("\tat %s.%s(", klass_name, method_name);

The comment about this line is useless and should be removed.

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

Changes requested by iklam (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/26894#pullrequestreview-3145411128
PR Review Comment: https://git.openjdk.org/jdk/pull/26894#discussion_r2294390668
PR Review Comment: https://git.openjdk.org/jdk/pull/26894#discussion_r2294370267
PR Review Comment: https://git.openjdk.org/jdk/pull/26894#discussion_r2294391604


More information about the hotspot-runtime-dev mailing list