8295851: Do not use ttyLock in BytecodeTracer::trace

src/hotspot/share/interpreter/bytecodeTracer.cpp

@@ -29,18 +29,14 @@
 #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,17 +81,22 @@ 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
   // the adjustments that BytecodeStream performs applies.
   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 +108,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 +140,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 +176,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

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");
+        }
+    }
 }