hpi-swa / MessageSendRecorder

Tracing tool for Squeak/Smalltalk. Includes a debugger-like browser for records.
MIT License
10 stars 2 forks source link

How to track an evaluable during recording? #2

Closed LinqLover closed 4 years ago

LinqLover commented 4 years ago

While recording some Morphic message send, I would like to track the value of a property of the morph. What is the best way to do so?

I experimented with #setBefore:after: and queried the property value in a block. But this was significantly slower than normal recording, slow enough for my image to hang up for hours and never complete; this even occurred when only requesting the new value if the receiver passed to the block was actually the morph of interest.

Btw: For best convenience, it would be great if one could trigger the MessageSendRecorder from a normal debugger and reuse the existing InspectorFields while debugging. But you probably already thought about that. :-)

Looking forward to your help! @marceltaeumel

marceltaeumel commented 4 years ago

Btw: For best convenience, it would be great if one could trigger the MessageSendRecorder from a normal debugger and reuse the existing InspectorFields while debugging. But you probably already thought about that. :-)

That's out-of-scope here. Tools can build on top of this project to add tracing, of course.

While recording some Morphic message send, I would like to track the value of a property of the morph. What is the best way to do so?

The record extension can hold values faster than dictionaries. An unknown accessor will be compiled into a new instance variable for MessageSendRecordExtension. So, you would think that this should work:

recorder watchPackageNamed: #Morphic.
recorder
    record: #normal
    before: {[:record :rcvr :args | record extension trackedColor: someMorph color]}
    after: #().

However, you have to look out for message sends that are currently recorded to avoid endless recursion. In this case, someMorph color is a problem. Similar to someMorph valueOfProperty: #myProperty. So, you should use the Squeak meta-object protocol:

recorder
    record: #normal
    before: {[:record :rcvr :args | record extension trackedColor: (morph instVarNamed: #color)]}
    after: #().

You can store that information outside of the record extension. Maybe reserve enough space upfront to avoid extra growth in that collection:

colors := OrderedCollection new: 100.
...
[:record :rcvr :args | colors add: (someMorph instVarNamed: #color)]
...
colors explore

You can also filter for the receiver:

[:record :rcvr :args |  rcvr == someMorph ifTrue: [colors add: (someMorph instVarNamed: #color)]]

But this was significantly slower than normal recording, slow enough for my image to hang up for hours and never complete

Well, you got into that endless recursion. It's not slow, it rather generates workload while trying to process that workload. Eventually, there will be an out-of-memory error or the VM will just crash. Mine just crashed.

LinqLover commented 4 years ago

Hi Marcel,

thank you very much for the helpful explanation! The API of the recorder is really great!

So, in a nutshell, the method wrappers are not removed during checking the before and after blocks. Why is this so? I cannot imagine any use case where recording the recorder code from the same recorder instance would be the desired behavior. Please find the attached changeset - I did not run any benchmarks against it, but the execution did not feel significantly slower: MsrDisableDuring.1.cs.zip

marceltaeumel commented 4 years ago

So, in a nutshell, the method wrappers are not removed during checking the before and after blocks. Why is this so?

Performance. You would need to uninstall all currently active method wrappers. Doing this for every message send before evaluating those recording blocks would be really slow.

Method wrappers are especially slow if you have to add a key to the respective method dictionary. For example, when wrapping a method in a subclass only defined in a super class.

LinqLover commented 4 years ago

Yes, but in my changeset I added a simple disable flag to the recorder, so it works in O(1). Are there any other obstacles? :-)

marceltaeumel commented 4 years ago

Your idea is to extend #shouldRecord to not only consider the current process but an extra flag, too. Well, you could just set recordingProcess to nil for the same effect. No extra instance variable required. Looks like a nice addition. Thanks! :-)

LinqLover commented 4 years ago

This was a MVP only. ;-) Glad you like it! I will open a PR right now ...

marceltaeumel commented 4 years ago

No need to. I will re-design that "disableDuring:", too, because the vocabulary does not match.

LinqLover commented 4 years ago

Alright :)