Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pretty thread names based on best-effort parsing of Thread#inspect output #54

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 19 additions & 1 deletion lib/vernier/output/firefox.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -385,6 +385,24 @@ def string_table

private

def pretty_name(name)
if name.empty?
begin
tr = ObjectSpace._id2ref(@ruby_thread_id)
name = tr.inspect if tr
rescue RangeError
# Thread was already GC'd
end
end
return name unless name.start_with?("#<Thread")
Copy link
Contributor Author

@dalehamel dalehamel Feb 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

case to cover "retained memory" name set for :retained mode, or if we ever got another name somehow. Also if we couldn't get any inspect output, no sense in doing the parsing below.

pretty = []
obj_address = name[/Thread:(0x\d+)/,1]
best_id = name[/\#<Thread:0x\w+@?\s?(.*)\s+\S+>/,1]
Copy link
Contributor Author

@dalehamel dalehamel Feb 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i originally wanted to do this just splitting on whitespace, but realized that this could get complicated very fast:

  • the thread name itself can contain spaces
  • the file path can contain spaces
  • both of these are optional

So instead, I try to just anchor on what we know will be present at the start (#<Thread:ADDRESS), and end ( STATE>)

If the thread was named, then the match for @? should catch this. Otherwise, try to just capture everything between the start and end anchors.

pretty << best_id unless best_id.empty?
pretty << "(#{obj_address})"
pretty.join(' ')
end

def gc_category
@categorizer.get_category("GC")
end
Expand Down
65 changes: 65 additions & 0 deletions test/output/test_firefox.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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