Best effort thread naming based on inspect output

Author: dalehamel

ext/vernier/vernier.cc

@@ -859,7 +859,6 @@ class Thread {
                     markers->record(Marker::Type::MARKER_GVL_THREAD_EXITED);
 
                     stopped_at = now;
-                    capture_name();
 
                     break;
             }
@@ -872,13 +871,6 @@ class Thread {
             return state != State::STOPPED;
         }
 
-        void capture_name() {
-            //char buf[128];
-            //int rc = pthread_getname_np(pthread_id, buf, sizeof(buf));
-            //if (rc == 0)
-            //    name = std::string(buf);
-        }
-
         void mark() {
         }
 };
@@ -945,8 +937,12 @@ class ThreadTable {
                     thread.set_state(new_state);
 
                     if (thread.state == Thread::State::RUNNING) {
+                        // rb_inspect should be safe here, as RUNNING should correspond to RESUMED hook from internal_thread_event_cb
+                        // which is called with GVL per https://github.com/ruby/ruby/blob/v3_3_0/include/ruby/thread.h#L247-L248
+                        VALUE thread_str  = rb_inspect(th);
                         thread.pthread_id = pthread_self();
                         thread.native_tid = get_native_thread_id();
+                        thread.name = StringValueCStr(thread_str);
                     } else {
                         thread.pthread_id = 0;
                         thread.native_tid = 0;
@@ -1496,13 +1492,6 @@ class TimeCollector : public BaseCollector {
         rb_remove_event_hook(internal_gc_event_cb);
         rb_remove_event_hook(internal_thread_event_cb);
 
-        // capture thread names
-        for (auto& thread: this->threads.list) {
-            if (thread.running()) {
-                thread.capture_name();
-            }
-        }
-
         frame_list.finalize();
 
         VALUE result = build_collector_result();

lib/vernier/output/firefox.rb

@@ -163,7 +163,7 @@ def initialize(ruby_thread_id, profile, categorizer, name:, tid:, samples:, weig
           @profile = profile
           @categorizer = categorizer
           @tid = tid
-          @name = name
+          @name = pretty_name(name)
 
           timestamps ||= [0] * samples.size
           @samples, @weights, @timestamps = samples, weights, timestamps
@@ -385,6 +385,20 @@ def string_table
 
         private
 
+        def pretty_name(name)
+          if name.empty?
+            tr = ObjectSpace._id2ref(@ruby_thread_id)
+            name = tr.inspect if tr
+          end
+          return name unless name.start_with?("#<Thread")
+          pretty = []
+          obj_address = name[/Thread:(0x\d+)/,1]
+          best_id = name[/\#<Thread:0x\w+@?\s?(.*)\s+\S+>/,1]
+          pretty << best_id unless best_id.empty?
+          pretty << "(#{obj_address})"
+          pretty.join(' ')
+        end
+
         def gc_category
           @categorizer.get_category("GC")
         end

test/output/test_firefox.rb

@@ -151,4 +151,69 @@ def test_custom_intervals
     markers = JSON.parse(output)["threads"].flat_map { _1["markers"]["data"] }
     assert_includes markers, {"type"=>"UserTiming", "entryType"=>"measure", "name"=>"custom"}
   end
+
+
+  def test_thread_names
+    orig_name = Thread.current.name
+    th1_loc, th2_loc = nil
+
+    # Case with just the named, main thread and no location
+    result = Vernier.trace do
+      Thread.current.name="main"
+    end
+
+    output = Vernier::Output::Firefox.new(result).output
+    assert_valid_firefox_profile(output)
+
+    data = JSON.parse(output)
+    threads = data["threads"]
+    assert_equal 1, threads.size
+    assert_match /^main \(0x\w+\)/, threads.first["name"]
+
+    # Case with unnamed thread and location
+    result = Vernier.trace do
+      th1 = Thread.new { th1_loc = file_lineno; sleep 0.01 }
+      th1.join
+    end
+
+    output = Vernier::Output::Firefox.new(result).output
+    assert_valid_firefox_profile(output)
+
+    data = JSON.parse(output)
+    threads = data["threads"]
+    assert_equal 2, threads.size
+
+    threads.each do |tr|
+      next if tr["isMainThread"]
+      assert_match /^#{th1_loc} \(0x\w+\)/, tr["name"]
+    end
+
+    # Case with named thread and location
+    result = Vernier.trace do
+      th2 = Thread.new { th2_loc = file_lineno; sleep 0.01 }
+      th2.name = "named thread"
+      th2.join
+    end
+
+    output = Vernier::Output::Firefox.new(result).output
+    assert_valid_firefox_profile(output)
+
+    data = JSON.parse(output)
+    threads = data["threads"]
+    assert_equal 2, threads.size
+
+    threads.each do |tr|
+      next if tr["isMainThread"]
+      assert_match /^named thread #{th2_loc} \(0x\w+\)/, tr["name"]
+    end
+
+  ensure
+    Thread.current.name = orig_name
+  end
+
+  private
+
+  def file_lineno
+    caller_locations(1, 1).first.yield_self{|loc| "#{loc.path}:#{loc.lineno}"}
+  end
 end