Skip check for pending breakpoints if no breakpoints are present

[WillHalto]

Description

Describe your changes:

• Calls to ThreadClient.current.on_load spend time pushing and popping the :load operation on the event queue, and waiting for the :load event to be handled.
• In a large Rails app with many loads, this extra time results a ~10-20% increase in environment load time when using require "debug", event when there are no breakpoints set at all. This seems to be at odd's with the README's "Fast: No performance penalty on non-stepping mode and non-breakpoints."
• By skipping calls to ThreadClient.current.on_load when there are no breakpoints present at all, we can save some of this time.
• As far as I can tell this is safe, as the main responsibility of ThreadClient.current.on_load is to handle any pending breakpoints in the loaded code, which we can safely skip if @bps is empty.

Example:

file_1.rb:

# file_1.rb
require "debug"

10000.times do
  load "./file_2.rb"
end

file_2.rb:

# file_2.rb
true

Before https://github.com/ruby/debug/commit/949562dda5b6ead980bc19540fed6a76452e0dea:

$ time bundle exec ruby file_1.rb

real    0m1.602s
user    0m1.044s
sys     0m0.690s

After https://github.com/ruby/debug/commit/949562dda5b6ead980bc19540fed6a76452e0dea:

$ time bundle exec ruby file_1.rb

real    0m0.835s
user    0m0.533s
sys     0m0.303s

^ It's a significant improvement in the overhead when using require "debug"

[st0012]

The on_load event also stores loaded files into the SourceRepository, which will later be used to display frames' file_lines. So skipping them will likely cause list command to return empty result.

[WillHalto]

The on_load event also stores loaded files into the SourceRepository, which will later be used to display frames' file_lines. So skipping them will likely cause list command to return empty result.

Gotcha, I tried this out locally but I wasn't able to see that behavior. list still works fine. It appears that for Ruby versions 3.1 and later, SourceRepository#add doesn't save any source info. It just relies on iseq.script_lines to get the lines.

For versions prior to 3.1, SourceRepository#add does store file lines, but skipping that appears to not be an issue either, as SourceRepository#get will fall back to adding them at retrieval time if they're not already stored.

Does that seem right? Apologies if I am missing something!

[st0012]

@WillHalto Yes I think you get it right. I missed the fallback mechanism.

[WillHalto]

Yes I think you get it right. I missed the fallback mechanism.

@st0012 thanks for the feedback! Based on that ^, does the approach in https://github.com/ruby/debug/commit/949562dda5b6ead980bc19540fed6a76452e0dea seem alright?

Alternatively, if we do want to preserve the calls to on_load even when there are no breakpoints, I wonder if we could directly call Session#on_load in the tracepoint instead of calling it via the thread client. I noticed there's a decent amount of time spent dealing with the :load events via the event queue:

==================================
  Mode: wall(1000)
  Samples: 2676 (7.40% miss rate)
  GC: 220 (8.22%)
==================================
     TOTAL    (pct)     SAMPLES    (pct)     FRAME
	 ...
      1355  (50.6%)         456  (17.0%)     Kernel#require
       221   (8.3%)         221   (8.3%)     Thread#join
       211   (7.9%)         211   (7.9%)     Thread::Queue#pop
       138   (5.2%)         138   (5.2%)     (marking)

which I don't believe is necessary. Perhaps we could instead try:

@tp_load_script = TracePoint.new(:script_compiled){|tp|
-    ThreadClient.current.on_load tp.instruction_sequence, tp.eval_script
+    on_load tp.instruction_sequence, tp.eval_script
}

and skip the event queue.

This works well in my tests and gives a similar (+/- ~100ms) performance improvement as the current proposed approach in the PR. Just something else to consider 👍

Before:

$ time bundle exec ruby file_1.rb

real    0m1.602s
user    0m1.044s
sys     0m0.690s

After change in https://github.com/ruby/debug/commit/949562dda5b6ead980bc19540fed6a76452e0dea

$ time bundle exec ruby file_1.rb

real    0m0.835s
user    0m0.533s
sys     0m0.303s

Directly calling on_load, skipping event queue:

time bundle exec ruby file_1.rb

real    0m0.998s
user    0m0.665s
sys     0m0.332s

[ko1]

Thank you for the idea.

• Before Ruby 3.1 source code repository is needed for eval'ed code and modified code (the code modified after loading).
• After Ruby 3.1, yes we can skip.

So I think we can skip it on Ruby 3.1 and later. Do you modify this PR?

[WillHalto]

@ko1 I've updated this in https://github.com/ruby/debug/pull/738/commits/0e12fa193a5b6c30ba714dfd0d053577d20163b3 to only skip it on Ruby 3.1 and later 👍

[ko1]

Thank you.