RFR: 8370044: TraceBytecodes shouldn't break up lines [v2]

[David Holmes]

On Fri, 20 Feb 2026 13:32:59 GMT, Paul Hübner <phubner at openjdk.org> wrote:

>> Hi all, 
>> 
>> This patch addresses bytecodes being torn/mangled when printing concurrently (i.e., tracing the bytecodes of concurrently executing threads). Taking `invokeinterface` on `foo` as an example, the output could look something like the following two, ranging from mangled but legible to completely illegible:
>> 
>> jint[27651] [27395]   696080    64  invokeinterface,  116 <CoherentBytecodeTraceTest$Strategy.foo(I)V> 
>> 
>> 
>> [43267]   696692    64  invokeinterface
>> )
>> [33283] [27395] [27907]   696694    29  fast_agetfield 116 <CoherentBytecodeTraceTest$Strategy.foo(I)V> 
>> 
>> 
>> The solution is to buffer each bytecode print into a `stringStream` before giving it to the tty. This avoids having to take a tty lock. With this solution, the print looks as follows (note: different occurrence so surrounding bytecodes differ):
>> 
>> [25347]   685730    61  iconst_1
>> [25091]   685717    64  invokeinterface 190 <CoherentBytecodeTraceTest$Strategy.foo(I)V> 
>> [24579]   685732    20  invokestatic 125 <java/util/concurrent/ConcurrentHashMap.spread(I)I> 
>> [24579]   685734    23  istore #4
>> 
>> 
>> I introduce a test case that finds `<CoherentBytecodeTraceTest$Strategy.foo(I)V>` and ensures the bytecode preceding it is correct. This reliably failed every time previously, and does not fail after my change.
>> 
>> Parsing the code, I noticed that bytecode printing takes a parameter `bool buffer = true` which internally buffer to a `stringStream` before flushing to the provided `outputStream`. There were numerous places where the calling code was performing buffering itself without passing `buffer = false`, effectively double buffering. This has been addressed.
>> 
>> Testing: tiers 1-4. on Linux (x64, AArch64), macOS (x64, AArch64), Windows (x64).
>
> Paul Hübner has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Johan's feedback.

Functional change looks good. I think I get the gist of the test.

A few minor style nits.

src/hotspot/share/code/nmethod.cpp line 941:

> 939:     ss.print_cr("implicit exception happened at " INTPTR_FORMAT, p2i(pc));
> 940:     print_on(&ss);
> 941:     // Buffering to a stringStream, disable internal buffering so it's not done twice

Suggestion:

    // Buffering to a stringStream, disable internal buffering so it's not done twice.

src/hotspot/share/interpreter/bytecodeTracer.hpp line 41:

> 39: class BytecodeTracer: AllStatic {
> 40:  public:
> 41:   NOT_PRODUCT(static void trace_interpreter(const methodHandle& method, address bcp, uintptr_t tos, uintptr_t tos2, outputStream* st);)

Seems an unnecessary change.

src/hotspot/share/runtime/java.cpp line 154:

> 152:           m->method_data()->parameters_type_data()->print_data_on(&ss);
> 153:         }
> 154:         // Buffering to a stringStream, disable internal buffering so it's not done twice

Suggestion:

        // Buffering to a stringStream, disable internal buffering so it's not done twice.

src/hotspot/share/runtime/vframe.inline.hpp line 179:

> 177:                   p2i(_frame.pc()), decode_offset);
> 178:       nm()->print_on(&ss);
> 179:       // Buffering to a stringStream, disable internal buffering so it's not done twice

Suggestion:

      // Buffering to a stringStream, disable internal buffering so it's not done twice.

test/hotspot/jtreg/runtime/interpreter/CoherentBytecodeTraceTest.java line 78:

> 76: 
> 77:     // The analysis works by finding the invokeinterface bytecode when calling
> 78:     // Strategy.foo The trace should look something like the following:

Suggestion:

    // Strategy.foo. The trace should look something like the following:

test/hotspot/jtreg/runtime/interpreter/CoherentBytecodeTraceTest.java line 80:

> 78:     // Strategy.foo The trace should look something like the following:
> 79:     // invokeinterface 116 <CoherentBytecodeTraceTest$Strategy.foo(I)V>
> 80:     // The stategy is to find CoherentBytecodeTraceTest$Strategy.foo's index

Suggestion:

    // The strategy is to find CoherentBytecodeTraceTest$Strategy.foo's index

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

PR Review: https://git.openjdk.org/jdk/pull/29842#pullrequestreview-3838709188
PR Review Comment: https://git.openjdk.org/jdk/pull/29842#discussion_r2838681890
PR Review Comment: https://git.openjdk.org/jdk/pull/29842#discussion_r2838681146
PR Review Comment: https://git.openjdk.org/jdk/pull/29842#discussion_r2838680844
PR Review Comment: https://git.openjdk.org/jdk/pull/29842#discussion_r2838680685
PR Review Comment: https://git.openjdk.org/jdk/pull/29842#discussion_r2838677901
PR Review Comment: https://git.openjdk.org/jdk/pull/29842#discussion_r2838678218