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

Replace WatchExprBreakpoint with WatchIVarBreakpoint #58

Merged
merged 5 commits into from Jun 8, 2021

Conversation

st0012
Copy link
Member

@st0012 st0012 commented Jun 2, 2021

Based on the discussion in #49, having WatchExprBreakpoint for watch all expression changes costs too much on performance but provide limited benefit to the users. But because watching instance variables for state changes is still valuable for debugging, we're going to support this particular case.

So in this PR, I made these changes:

  • Replace WatchExprBreakpoint with WatchIVarBreakpoint.
  • watch expr is no longer supported.
  • watch @ivar is more accurate and cheaper.

About WatchIVarBreakpoint

Currently, when watching instance variables, the breakpoint isn't aware of the instance/class it's tracking on. So it could incorrectly track instance variable change (with the same name) in a different class.

For example, if you debug this script:

class Student
  attr_accessor :name

  def initialize(name)
    @name = name
    binding.bp(command: "watch @name ;; c") # should only watch Student's @name change
  end
end

class Teacher
  attr_accessor :name

  def initialize(name)
    @name = name
  end
end

s1 = Student.new("John")
t1 = Teacher.new("Jane") # this shouldn't trigger a stop
s1.name = "Josh"
"foo"

You'll find that it stops at Teacher's @name assignments as well, even though the script should only monitor Student's @name.

✦ ❯ exe/rdbg target.rb
[1, 10] in target.rb
=>    1| class Student
      2|   attr_accessor :name
      3|
      4|   def initialize(name)
      5|     @name = name
      6|     binding.bp(command: "watch @name ;; c")
      7|   end
      8| end
      9|
     10| class Teacher
=>#0    <main> at target.rb:1

(rdbg) c
[1, 10] in target.rb
      1| class Student
      2|   attr_accessor :name
      3|
      4|   def initialize(name)
      5|     @name = name
=>    6|     binding.bp(command: "watch @name ;; c")
      7|   end
      8| end
      9|
     10| class Teacher
=>#0    Student#initialize(name="John") at target.rb:6
  #1    [C] Class#new at target.rb:18
  #2    <main> at target.rb:18
(rdbg:init) watch @name
@name = John
#0 watch bp: @name = John
(rdbg:init) c
[14, 22] in target.rb
     14|     @name = name
     15|   end
     16| end
     17|
     18| s1 = Student.new("John")
=>   19| t1 = Teacher.new("Jane")
     20| s1.name = "Josh"
     21| "foo"
     22|
=>#0    <main> at target.rb:19

Stop by #0 watch bp: @name = John ->

(rdbg) c
[10, 19] in target.rb
     10| class Teacher
     11|   attr_accessor :name
     12|
     13|   def initialize(name)
     14|     @name = name
=>   15|   end
     16| end
     17|
     18| s1 = Student.new("John")
     19| t1 = Teacher.new("Jane")
=>#0    Teacher#initialize(name="Jane") at target.rb:15 #=> "Jane"
  #1    [C] Class#new at target.rb:19
  #2    <main> at target.rb:19

Stop by #0 watch bp: @name =  -> Jane

(rdbg) c
[15, 22] in target.rb
     15|   end
     16| end
     17|
     18| s1 = Student.new("John")
     19| t1 = Teacher.new("Jane")
=>   20| s1.name = "Josh"
     21| "foo"
     22|
=>#0    <main> at target.rb:20

Stop by #0 watch bp: @name = Jane ->

(rdbg) c

After

✦ ❯ exe/rdbg target.rb
[1, 10] in target.rb
=>    1| class Student
      2|   attr_accessor :name
      3|
      4|   def initialize(name)
      5|     @name = name
      6|     binding.bp(command: "watch @name ;; c")
      7|   end
      8| end
      9|
     10| class Teacher
=>#0    <main> at target.rb:1

(rdbg) c
[1, 10] in target.rb
      1| class Student
      2|   attr_accessor :name
      3|
      4|   def initialize(name)
      5|     @name = name
=>    6|     binding.bp(command: "watch @name ;; c")
      7|   end
      8| end
      9|
     10| class Teacher
=>#0    Student#initialize(name="John") at target.rb:6
  #1    [C] Class#new at target.rb:18
  #2    <main> at target.rb:18
(rdbg:init) watch @name
#<Student:0x00007fc3c68cc8c8> @name = John
#0 watch bp: #<Student:0x00007fc3c68cc8c8> @name = John
(rdbg:init) c
[16, 22] in target.rb
     16| end
     17|
     18| s1 = Student.new("John")
     19| t1 = Teacher.new("Jane")
     20| s1.name = "Josh"
=>   21| "foo"
     22|
=>#0    <main> at target.rb:21

Stop by #0 watch bp: #<Student:0x00007fc3c68cc8c8> @name = John -> Josh

(rdbg) c

Another benefit of the change is that we can show what instance's ivar is changed:

#<Student:0x00007fc3c68cc8c8> @name = John

(I plan to work on styling it in another PR)

@st0012 st0012 mentioned this pull request Jun 2, 2021
test/debug/watch_test.rb Outdated Show resolved Hide resolved
@ko1
Copy link
Collaborator

ko1 commented Jun 3, 2021

This proposal is strongly connected with "how to design watch command". #49

So wait for merging.

@st0012 st0012 changed the title Improve instance variable watching's accuracy Replace WatchExprBreakpoint with WatchIVarBreakpoint Jun 4, 2021
@st0012
Copy link
Member Author

st0012 commented Jun 4, 2021

@ko1 btw this breakpoint is immune from the issue I mentioned in #61 (comment)

lib/debug/thread_client.rb Outdated Show resolved Hide resolved
target.rb Outdated Show resolved Hide resolved
@st0012 st0012 requested a review from ko1 June 4, 2021 05:55
@st0012 st0012 force-pushed the improve-watch branch 2 times, most recently from e5d0b96 to d6477ff Compare June 7, 2021 16:17
@ko1 ko1 merged commit 83d9333 into ruby:master Jun 8, 2021
@st0012 st0012 deleted the improve-watch branch June 8, 2021 07:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants