From 29b71ba5c0706883d482a8a55d37082ee4e6ea74 Mon Sep 17 00:00:00 2001 From: Coleen Phillimore Date: Tue, 17 Jun 2025 22:05:34 +0000 Subject: [PATCH 1/3] 8295851: Do not use ttyLock in BytecodeTracer::trace --- .../share/interpreter/bytecodeTracer.cpp | 46 ++++++++----------- .../runtime/interpreter/TraceBytecodes.java | 39 ++++++++++++++-- 2 files changed, 54 insertions(+), 31 deletions(-) diff --git a/src/hotspot/share/interpreter/bytecodeTracer.cpp b/src/hotspot/share/interpreter/bytecodeTracer.cpp index 798c0bfc7d81c..8e847c0bbe52a 100644 --- a/src/hotspot/share/interpreter/bytecodeTracer.cpp +++ b/src/hotspot/share/interpreter/bytecodeTracer.cpp @@ -29,18 +29,13 @@ #include "interpreter/bytecodeTracer.hpp" #include "interpreter/bytecodes.hpp" #include "interpreter/interpreter.hpp" -#include "interpreter/interpreterRuntime.hpp" #include "memory/resourceArea.hpp" #include "oops/constantPool.inline.hpp" #include "oops/methodData.hpp" #include "oops/method.hpp" -#include "oops/resolvedFieldEntry.hpp" -#include "oops/resolvedIndyEntry.hpp" -#include "oops/resolvedMethodEntry.hpp" +#include "runtime/atomic.hpp" #include "runtime/handles.inline.hpp" #include "runtime/mutexLocker.hpp" -#include "runtime/osThread.hpp" -#include "runtime/timer.hpp" #include "utilities/align.hpp" // Prints the current bytecode and its attributes using bytecode-specific information. @@ -85,10 +80,11 @@ class BytecodePrinter { void bytecode_epilog(int bci, outputStream* st); public: - BytecodePrinter(int flags = 0) { - _is_wide = false; - _code = Bytecodes::_illegal; - _flags = flags; + BytecodePrinter(int flags = 0) : _is_wide(false), _code(Bytecodes::_illegal), _flags(flags) {} + +#ifndef PRODUCT + BytecodePrinter(Method* prev_method) : BytecodePrinter(0) { + _current_method = prev_method; } // This method is called while executing the raw bytecodes, so none of @@ -96,6 +92,10 @@ class BytecodePrinter { void trace(const methodHandle& method, address bcp, uintptr_t tos, uintptr_t tos2, outputStream* st) { ResourceMark rm; bool method_changed = _current_method != method(); + _current_method = method(); + _is_linked = method->method_holder()->is_linked(); + assert(_is_linked, "this function must be called on methods that are already executing"); + if (method_changed) { // Note 1: This code will not work as expected with true MT/MP. // Need an explicit lock or a different solution. @@ -107,9 +107,6 @@ class BytecodePrinter { st->print("[%zu] ", Thread::current()->osthread()->thread_id_for_printing()); method->print_name(st); st->cr(); - _current_method = method(); - _is_linked = method->method_holder()->is_linked(); - assert(_is_linked, "this function must be called on methods that are already executing"); } Bytecodes::Code code; if (is_wide()) { @@ -142,14 +139,13 @@ class BytecodePrinter { _is_wide = (code == Bytecodes::_wide); _code = Bytecodes::_illegal; -#ifndef PRODUCT if (TraceBytecodesStopAt != 0 && BytecodeCounter::counter_value() >= TraceBytecodesStopAt) { TraceBytecodes = false; } -#endif } +#endif - // Used for Method*::print_codes(). The input bcp comes from + // Used for Method::print_codes(). The input bcp comes from // BytecodeStream, which will skip wide bytecodes. void trace(const methodHandle& method, address bcp, outputStream* st) { _current_method = method(); @@ -179,19 +175,17 @@ class BytecodePrinter { }; #ifndef PRODUCT -// We need a global instance to keep track of the states when the bytecodes -// are executed. Access by multiple threads are controlled by ttyLocker. -static BytecodePrinter _interpreter_printer; +// We need a global instance to keep track of the method being printed so we can report that +// the method has changed. If this method is redefined and removed, that's ok because the method passed in won't match, and +// this will print that one. +static Method* _current_method = nullptr; void BytecodeTracer::trace_interpreter(const methodHandle& method, address bcp, uintptr_t tos, uintptr_t tos2, outputStream* st) { if (TraceBytecodes && BytecodeCounter::counter_value() >= TraceBytecodesAt) { - ttyLocker ttyl; // 5065316: keep the following output coherent - // The ttyLocker also prevents races between two threads - // trying to use the single instance of BytecodePrinter. - // - // There used to be a leaf mutex here, but the ttyLocker will - // work just as well, as long as the printing operations never block. - _interpreter_printer.trace(method, bcp, tos, tos2, st); + BytecodePrinter printer(_current_method); + printer.trace(method, bcp, tos, tos2, st); + // Save current method to detect when method printing changes. + Atomic::release_store(&_current_method, method()); } } #endif diff --git a/test/hotspot/jtreg/runtime/interpreter/TraceBytecodes.java b/test/hotspot/jtreg/runtime/interpreter/TraceBytecodes.java index b599f6c61ff04..417e3a32b4bde 100644 --- a/test/hotspot/jtreg/runtime/interpreter/TraceBytecodes.java +++ b/test/hotspot/jtreg/runtime/interpreter/TraceBytecodes.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2023, 2025, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -26,13 +26,42 @@ * @bug 8309811 * @requires vm.debug * @summary Test the output of -XX:+TraceBytecodes, -XX:TraceBytecodesAt, and -XX:TraceBytecodesStopAt - * @run main/othervm -XX:+TraceBytecodes -XX:TraceBytecodesAt=2000 -XX:TraceBytecodesStopAt=3000 TraceBytecodes + * @run main/othervm -XX:+TraceBytecodes -XX:TraceBytecodesAt=1000000 -XX:TraceBytecodesStopAt=1001000 TraceBytecodes */ -// This is just a very simple sanity test. Trace about 1000 bytecodes. See the .jtr file for the output. +// CountBytecodes returns 1826384 bytecodes, so trace starting at a million to trace parallel threads. +// This is just a very simple sanity test. See the .jtr file for the output. // Consider it OK if the VM doesn't crash. It should test a fair amount of the code in bytecodeTracer.cpp -public class TraceBytecodes { - public static void main(String args[]) { +public class TraceBytecodes extends Thread { + public void run() { System.out.println("Hello TraceBytecodes"); } + + private static TraceBytecodes[] threads = new TraceBytecodes[2]; + private static boolean success = true; + + private static boolean report_success() { + for (int i = 0; i < 2; i++) { + try { + threads[i].join(); + } catch (InterruptedException e) {} + } + return success; + } + + public static void main(String args[]) { + threads[0] = new TraceBytecodes(); + threads[1] = new TraceBytecodes(); + for (int i = 0; i < 2; i++) { + threads[i].setName("Loading Thread #" + (i + 1)); + threads[i].start(); + System.out.println("Thread " + (i + 1) + " was started..."); + } + + if (report_success()) { + System.out.println("PASSED"); + } else { + throw new RuntimeException("FAILED"); + } + } } From 0e4421bee80865744e944bfcbe90840c60fa9144 Mon Sep 17 00:00:00 2001 From: Coleen Phillimore Date: Fri, 20 Jun 2025 12:48:26 +0000 Subject: [PATCH 2/3] Put back include osthread.hpp --- src/hotspot/share/interpreter/bytecodeTracer.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/hotspot/share/interpreter/bytecodeTracer.cpp b/src/hotspot/share/interpreter/bytecodeTracer.cpp index 8e847c0bbe52a..ef3057c808748 100644 --- a/src/hotspot/share/interpreter/bytecodeTracer.cpp +++ b/src/hotspot/share/interpreter/bytecodeTracer.cpp @@ -36,6 +36,7 @@ #include "runtime/atomic.hpp" #include "runtime/handles.inline.hpp" #include "runtime/mutexLocker.hpp" +#include "runtime/osthread.hpp" #include "utilities/align.hpp" // Prints the current bytecode and its attributes using bytecode-specific information. From 5cdc5e11422b3a008e68d69c0c86876187185724 Mon Sep 17 00:00:00 2001 From: Coleen Phillimore Date: Fri, 20 Jun 2025 13:26:47 +0000 Subject: [PATCH 3/3] Put back include osthread.hpp --- src/hotspot/share/interpreter/bytecodeTracer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hotspot/share/interpreter/bytecodeTracer.cpp b/src/hotspot/share/interpreter/bytecodeTracer.cpp index ef3057c808748..6498acb8073a8 100644 --- a/src/hotspot/share/interpreter/bytecodeTracer.cpp +++ b/src/hotspot/share/interpreter/bytecodeTracer.cpp @@ -36,7 +36,7 @@ #include "runtime/atomic.hpp" #include "runtime/handles.inline.hpp" #include "runtime/mutexLocker.hpp" -#include "runtime/osthread.hpp" +#include "runtime/osThread.hpp" #include "utilities/align.hpp" // Prints the current bytecode and its attributes using bytecode-specific information.