From c3908723be36ad5c88c0376fec90e9c3bbc008cc Mon Sep 17 00:00:00 2001 From: Koichi Sasada Date: Mon, 11 Jul 2022 17:13:41 +0900 Subject: [PATCH] Improve method breakpoint performance. With pending method breakpoints, track all method invocation to track `method_added` and `singleton_method_added`. Of course, this approach slows down the debuggee's performance. This patch tracks all `method_added` and `singleton_method_added` methods and further defined such methods for the classes/modules. This approach doesn't have performance penalty any more. This patch also fix the message at registering the pending method breakpoint. ```ruby binding.b do: 'b C#foo' unless ENV['NO_PENDING_BP'] def fib n if n<2 n else fib(n-1)+fib(n-2) end end require 'benchmark' Benchmark.bm{|x| x.report{ fib(35) } } ``` Results: ``` Without pending breakpoints: $ NO_PENDING_BP=1 exe/rdbg -n target.rb user system total real 0.929580 0.000000 0.929580 ( 0.929682) With pending breakpoints - Before this patch: $ exe/rdbg -n target.rb [1, 10] in target.rb => 1| binding.b do: <<~DBG 2| b C#foo 3| DBG 4| 5| def fib n 6| if n<2 7| n 8| else 9| fib(n-1)+fib(n-2) 10| end =>#0
at target.rb:1 (rdbg:binding.break) b C#foo uninitialized constant C C ^ user system total real 3.276217 0.000000 3.276217 ( 3.276299) With pending breakpoints - After this patch: [master]$ exe/rdbg -n target.rb [1, 10] in target.rb => 1| binding.b do: <<~DBG 2| b C#foo 3| DBG 4| 5| def fib n 6| if n<2 7| n 8| else 9| fib(n-1)+fib(n-2) 10| end =>#0
at target.rb:1 (rdbg:binding.break) b C#foo Unknown name `C` for `Object` user system total real 0.933402 0.002353 0.935755 ( 0.935757) ``` --- ext/debug/debug.c | 22 ------------- lib/debug/session.rb | 63 ++++++++++++++++++++++++++------------ lib/debug/thread_client.rb | 5 ++- 3 files changed, 48 insertions(+), 42 deletions(-) diff --git a/ext/debug/debug.c b/ext/debug/debug.c index de8fe455a..81c960541 100644 --- a/ext/debug/debug.c +++ b/ext/debug/debug.c @@ -97,27 +97,6 @@ frame_depth(VALUE self) return INT2FIX(RARRAY_LEN(bt)); } -static void -method_added_tracker(VALUE tpval, void *ptr) -{ - rb_trace_arg_t *arg = rb_tracearg_from_tracepoint(tpval); - VALUE mid = rb_tracearg_callee_id(arg); - - if (RB_UNLIKELY(mid == ID2SYM(rb_intern("method_added")) || - mid == ID2SYM(rb_intern("singleton_method_added")))) { - VALUE args[] = { - tpval, - }; - rb_funcallv(rb_mDebugger, rb_intern("method_added"), 1, args); - } -} - -static VALUE -create_method_added_tracker(VALUE self) -{ - return rb_tracepoint_new(0, RUBY_EVENT_CALL, method_added_tracker, NULL); -} - // iseq const struct rb_iseq *rb_iseqw_to_iseq(VALUE iseqw); @@ -203,7 +182,6 @@ Init_debug(void) rb_gc_register_mark_object(rb_cFrameInfo); rb_define_singleton_method(rb_mDebugger, "capture_frames", capture_frames, 1); rb_define_singleton_method(rb_mDebugger, "frame_depth", frame_depth, 0); - rb_define_singleton_method(rb_mDebugger, "create_method_added_tracker", create_method_added_tracker, 0); rb_define_const(rb_mDebugger, "SO_VERSION", rb_str_new2(RUBY_DEBUG_VERSION)); // iseq diff --git a/lib/debug/session.rb b/lib/debug/session.rb index e3126b753..d9940efc5 100644 --- a/lib/debug/session.rb +++ b/lib/debug/session.rb @@ -1643,7 +1643,7 @@ def method_added tp b = tp.binding if var_name = b.local_variables.first mid = b.local_variable_get(var_name) - unresolved = false + resolved = true @bps.each{|k, bp| case bp @@ -1654,15 +1654,53 @@ def method_added tp end end - unresolved = true unless bp.enabled? + resolved = false if !bp.enabled? end } - unless unresolved - METHOD_ADDED_TRACKER.disable + + if resolved + Session.deactivate_method_added_trackers + end + + case mid + when :method_added, :singleton_method_added + Session.create_method_added_tracker(tp.self, mid) + Session.create_method_added_tracker unless resolved end end end + class ::Module + undef method_added + def method_added mid; end + def singleton_method_added mid; end + end + + def self.create_method_added_tracker mod, method_added_id, method_accessor = :method + m = mod.__send__(method_accessor, method_added_id) + METHOD_ADDED_TRACKERS[m] = TracePoint.new(:call) do |tp| + SESSION.method_added tp + end + end + + def self.activate_method_added_trackers + METHOD_ADDED_TRACKERS.each do |m, tp| + tp.enable(target: m) unless tp.enabled? + rescue ArgumentError + DEBUGGER__.warn "Methods defined under #{m.owner} can not track by the debugger." + end + end + + def self.deactivate_method_added_trackers + METHOD_ADDED_TRACKERS.each do |m, tp| + tp.disable if tp.enabled? + end + end + + METHOD_ADDED_TRACKERS = Hash.new + create_method_added_tracker Module, :method_added, :instance_method + create_method_added_tracker Module, :singleton_method_added, :instance_method + def width @ui.width end @@ -2076,21 +2114,8 @@ def self.load_rc end end - class ::Module - undef method_added - def method_added mid; end - def singleton_method_added mid; end - end - - def self.method_added tp - begin - SESSION.method_added tp - rescue Exception => e - p e - end - end - - METHOD_ADDED_TRACKER = self.create_method_added_tracker + # Inspector + SHORT_INSPECT_LENGTH = 40 class LimitedPP diff --git a/lib/debug/thread_client.rb b/lib/debug/thread_client.rb index 65c526309..ea014b618 100644 --- a/lib/debug/thread_client.rb +++ b/lib/debug/thread_client.rb @@ -744,9 +744,12 @@ def make_breakpoint args bp = MethodBreakpoint.new(current_frame.eval_binding, klass_name, op, method_name, cond: cond, command: cmd, path: path) begin bp.enable + rescue NameError => e + puts "Unknown name `#{e.name}` for `#{e.receiver}`" + # TODO: Ractor support + Session.activate_method_added_trackers rescue Exception => e puts e.message - ::DEBUGGER__::METHOD_ADDED_TRACKER.enable end bp