Add an option for hiding anonymous threads in profile output#175
Add an option for hiding anonymous threads in profile output#175tenderlove wants to merge 1 commit into
Conversation
This commit adds an option for initially hiding anonymous threads in profile output. This is particularly useful for profiling RubyGems / Bundler because most threads that do real work have names associated with them. However, some routines like the popen family will [spawn a watchdog thread](https://github.com/ruby/ruby/blob/04e90fe200d736db0a32a794b8dc742fa0cb5441/lib/open3.rb#L535), and these watchdog threads clutter the profile output.
joshuay03
left a comment
joshuay03
left a comment
There was a problem hiding this comment.
Thanks! Some minor things
| end | ||
|
|
||
| initially_visible_threads = if hide_anonymous_threads | ||
| threads.each_index.reject { |i| threads[i].anonymous? && !threads[i].is_main }.to_a |
There was a problem hiding this comment.
This guards is_main but not is_start. There's already a test (test_selected_thread_with_spawned_thread_start) covering profiling that begins on a non-main thread where the starting thread has is_main = false and is_start = true. If that thread has no name, hide_anonymous_threads: true would exclude it from initialVisibleThreads even though it's the thread in initialSelectedThreads. The user would open the profile and their thread of interest would be hidden.
| threads.each_index.reject { |i| threads[i].anonymous? && !threads[i].is_main }.to_a | |
| threads.each_index.reject { |i| threads[i].anonymous? && !threads[i].is_main && !threads[i].is_start }.to_a |
| assert result.meta[:hide_anonymous_threads] | ||
|
|
||
| output = JSON.parse(Vernier::Output::Firefox.new(result).output) | ||
| assert_equal 2, output["meta"]["initialVisibleThreads"].length |
There was a problem hiding this comment.
This checks count but not identity; it would pass even if the wrong two threads were visible. Worth asserting that the named thread is included and the anonymous one isn't, e.g.:
| assert_equal 2, output["meta"]["initialVisibleThreads"].length | |
| visible_names = output["meta"]["initialVisibleThreads"].map { |i| output["threads"][i]["name"] } | |
| assert_includes visible_names, "not anonymous" | |
| anonymous_thread = output["threads"].find { |t| t["name"] != "not anonymous" && !t["isMainThread"] } | |
| refute_includes visible_names, anonymous_thread.fetch("name") |
There was a problem hiding this comment.
Might also be worth adding a case that mirrors test_selected_thread_with_spawned_thread_start where profiling started from an unnamed non-main thread and hide_anonymous_threads: true should still show that thread.
2 participants
This commit adds an option for initially hiding anonymous threads in profile output. This is particularly useful for profiling RubyGems / Bundler because most threads that do real work have names associated with them. However, some routines like the popen family will spawn a watchdog thread, and these watchdog threads clutter the profile output.
Before this patch:
After this patch:
Watchdog threads are still available from the "tracks" dropdown (if you want to see them):