hpi-swa / MessageSendRecorder

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

Recording of iterative message sends is wrong/counterintuitive #5

Open LinqLover opened 4 years ago

LinqLover commented 4 years ago

Steps to reproduce:

m := Morph new.
3 timesRepeat: [m addMorph: Morph new].

MessageSendRecorder new
    watchPackageNamed: #Morphic;
    setMessageSend: [m submorphsDo: [:n | n show]];
    record;
    browse.

Output: image

The representation is counterintuitive. When using #recordAllObjects, the argument n is not recorded either.

Rather, I would expect to see three children of the Morph>>submorphsDo: node, one for each invocation of the [] in UndefinedObject>>DoIt block.

LinqLover commented 4 years ago

Unfortunately, the number of nested nodes does not even correspond with the number of iterations but the number of calls from the iterator block ...

image

marceltaeumel commented 3 years ago

Interesting. Well, it is not possible to catch neither block evaluation nor its return. That []-record is created manually. You discovered the drawback of that.

See: https://github.com/hpi-swa/MessageSendRecorder/blob/master/packages/MessageSendRecorder.package/MessageSendRecorder.class/instance/recordSendId.method.receiver.arguments.context..st

Would be nice to also simulate the block return somehow.

LinqLover commented 3 years ago

Hm, in theory, this should be solved by checking #outerCode of the right CompiledBlock, shouldn't it? And does MSR support the V3.X bytecode set all?

marceltaeumel commented 3 years ago

I don't think that I use SistaV1-specific calls. should work with V3PlusClosures, too.

LinqLover commented 3 years ago

But this refers to SistaV1 explicitly:

https://github.com/hpi-swa/MessageSendRecorder/blob/befc38911ab66cc810aaa03539ae7b6d8374c0b3/packages/MessageSendRecorder.package/MessageSendRecorder.class/instance/recordSendId.method.receiver.arguments.context..st#L10

In V3PlusClosures, we don't have CompiledBlocks, so aContext method will always equal closure homeMethod, correctly?

marceltaeumel commented 3 years ago

Correct. So, where is the issue? :-) Just the comment refers to SistaV1, not the code.

LinqLover commented 3 years ago

FWIW, here is another erroneous record that assumes recursion instead of method calls:

image

Recorded watching the packages #(Metacello Gofer) with #recordAllObjects.