Improve the watch command

[st0012]

Description copied from #41:

I found several things when playing with the watch command:

• It mainly uses line events to detect the change instead of the return events (I'm not sure why though). This causes the program to always stop a line after the change. So in tests we need to add an extra line after the changed line. Otherwise it doesn't stop.
• It always evaluates the expression from top-level scope. So it's not able to track an instance variable inside an object like watch @name and users need to use watch obj.name instead. I think we should improve this.

[ko1]

Otherwise it doesn't stop.

Could you show the example?

I agree it should stop also return events, but it will stops just another frames.
or another frame can't evaluate because of different bindings?

I think we should improve this.

Maybe this request is, "I want to watch the change of an ivar of THIS object". So it needs to specify two parameters, "Object" and "Ivar name".

I'm planning to introduce watch @ivar of obj. Does it make sense?
Current watch command only support "global visible" variables.

Another topic of "watch" command is, should we keep the binding of the frame.
If we keep the binding of "watch" command, we can evaluate "@ivar" of binding, and you don't need to specify "of self". It seems convenient, but it seems too much feature. No conclusion.

[st0012]

Could you show the example?

if the change happens at the last line if the program, it doesn't stop there. that's why the tests need an extra line at the end

debug/test/debug/watch_test.rb

Lines 8 to 14 in f44de3a

	<<~RUBY
	a = 1
	b = 2
	c = 3
	a = 2
	foo = "foo" # stops here
	RUBY

I'm planning to introduce watch @ivar of obj. Does it make sense?

yeah this feature sounds good 👍 but syntax-wise I'd go for watch obj @ivar.

Maybe this request is, "I want to watch the change of an ivar of THIS object". So it needs to specify two parameters, "Object" and "Ivar name".

from a user's perspective, if the breakpoint is already inside an object's method, I'd expect watch @var to just work. for example:

class Foo
  def initialize
    @var = 0
    binding.bp(command: "watch @var")
  end

  def bar
    @var = 10
  end
end

f = Foo.new
f.bar

I know this could be hard to implement and may not a be priority right now. but I believe it provides a better user experience.

[ko1]

or only allow for ivar (or gvar) like watch @ivar ?
It's simple. Nobody can misuse local_var.

[st0012]

so only:

• watch @ivar
• watch $gvar

but no watch local?
what if the breakpoint is inside an instance and I want to watch its method like watch name (for monitor its return value, not if it's called)? can it distinguish the difference?

[ko1]

no watch local.

The difference is, allow any expression or not.

[st0012]

maybe we can preserve watch local while also improving watch @ivar? I've given it a try in #58.

[ko1]

Maybe we need a scenario to design this feature.
Can you list it?

[st0012]

@ko1 do you mean watch @ivar or watch local?

I don't think I'd use watch local very often so I'm ok with dropping it. I just think it'd be great if we can keep it while improving watch @ivar.

as of watch @ivar, I'd mainly use it for tracking for variable mutation from the class's source.

for example, tracking down what changes a Student instance's name:

class Student
  attr_accessor :name

  def initialize(name)
    @name = name
  end
end

# somewhere else

student = Student.new("John")
SomeService.perform(student)

without the debugger, it'd take several steps:

1. diving into the source code of SomeService.
2. search for name = or `name +=...etc.
3. see if the student object is passed into somewhere else.
4. if there are multiple places that can perform the update, place multiple puts or binding.irb to verify each of them.

but with debugger & watch @ivar, I can just do

class Student
  attr_accessor :name

  def initialize(name)
    @name = name
    binding.bp(command: "watch @name ;; c")
  end
end

then it'll stop at the places where @name is being updated.

and it's even more convenient than invoking TracePoint manually, because it can't trace methods generated by attr_* helpers.

[st0012]

btw I think watch local could be useful in places like this: (watch start_line & watch end_line)

debug/lib/debug/thread_client.rb

Lines 248 to 265 in 9d7b88a

	unless start_line
	if frame.show_line
	if dir > 0
	start_line = frame.show_line
	else
	end_line = frame.show_line - max_lines
	start_line = [end_line - max_lines, 0].max
	end
	else
	start_line = [frame_line - max_lines/2, 0].max
	end
	end
	unless end_line
	end_line = [start_line + max_lines, lines.size].min
	end

[ko1]

How about general scenario other than ivars?

---

btw I think watch local could be useful in places like this: (watch start_line & watch end_line)

The lifetime of local variables are enough small, so I think it is not so important for lvars (if we make a Proc, the lifetime is not limited to the scope though).

[st0012]

The lifetime of local variables are enough small, so I think it is not so important for lvars (if we make a Proc, the lifetime is not limited to the scope though).

@ko1 this is generally the case, but I have some real-world examples for the use of watch local:

The code below comes from a library I use at work and have debugged before:

https://github.com/cerebris/jsonapi-resources/blob/8cdc9dd5b8f29f70a0cfe436840ac8033f0cdad3/lib/jsonapi/relationship_builder.rb#L56-L80

As you can see, the records local is assigned multiple times under different conditions. When I debugged it, I had to put a binding.irb or binding.pry at every records =. But with binding.bp(command: "watch records ;; c") at the top of the method, the debugger would just stop itself. I think that'd be pretty neat.

(although I should note that watch local is sometimes inaccurate in my application. but we can improve that later).

And here's another example from the same library: https://github.com/cerebris/jsonapi-resources/blob/release-0-9/lib/jsonapi/resource.rb#L1246, which has an even longer method and more local assignments.

Although these cases are not common, they're where this debugger can help the most.

[st0012]

Regarding other watch expr cases, I can only come up with:

• $global - useful for monitoring global state changes, which is sometimes used in test/dev environment.
• obj.method - it's helpful when debugging with a class' public interfaces. for example, when debugging an ActiveRecord instance, it's easier to debug with post.author_name than watching its instance variables.

[ko1]

But with binding.bp(command: "watch records ;; c") at the top of the method, the debugger would just stop itself. I think that'd be pretty neat.

• It makes many watch points (calling the method, making a watch point). And watch points introduce huge overhead. 1 watch makes xN times slower.
• To support lvar (or any Ruby syntax on the current frame, like obj.foo), we need to keep the binding and it will not be freed.

But for the debug purpose, it is acceptable...?

[ko1]

I'm afraid that people ask me to make it fast.

[ko1]

For ivar, I will introduce TracePoint for ivars setting.
For local variables, it should not introduced because of normal performance.
Also obj.foo notation we can't introduce TP and impossible to support lightweight implementation.

[st0012]

To support lvar (or any Ruby syntax on the current frame, like obj.foo), we need to keep the binding and it will not be freed.

I see. Given the performance penalty I think it's reasonable to drop it.

For ivar, I will introduce TracePoint for ivars setting.

I guess the support will come in Ruby 3.1? In that case, will we keep 2 implementations in the debugger? One for 3.1+ and the other for 2.6~3.0

[ko1]

Another idea is, shows a warning message "it will make your program super slow" for adding watch points, and doesn't show it ivar on 3.1 with TP optimization.

[ko1]

I guess the support will come in Ruby 3.1? In that case, will we keep 2 implementations in the debugger?

For watch points, Yes.

[st0012]

@ko1 cool sounds good 👍 I think my PR has improved the ivar case then.

[st0012]

@ko1 since we're going to drop expression bp anyway, how about let's just do it in #58?

[ko1]

Close this ticket and discuss more features in another issue/PR.