Alternative tailRecM implementation #57
Conversation
|
I'm about to go out so don't have time to review this right now, but I found SD-1665 horrifying when I read about it this morning, so I'm glad it seems like there might be a reasonable way out of it. |
natefaubion
force-pushed
the
monadrec-sync-bounce
branch
from
a520cb6 to
859a596
Compare
|
I have pushed some inline comments to hopefully better explain what is going on. |
| // status = 2 (asynchronous effect) | ||
| // | ||
| // The inner Aff is run, and if the status is still 0, we have observed | ||
| // a synchronous effect. The result is stashed for later synchronous |
There was a problem hiding this comment.
Can you explain why the status being still 0 means we've observed a synchronous effect? I'm sure you're right, but since I don't really know the guts of Aff too well, it wasn't quite clear to me how.
There was a problem hiding this comment.
The order of synchronous effects are:
- Initialize status to 0
- Run Aff
- Observe status
If the status changed between initializing and observing it, our Aff has resolved synchronously. When resolving synchronously, we check that the status has NOT changed from its initial state. You have to observe the change in two places: during resolution and, after launching the Aff.
There was a problem hiding this comment.
OK, thanks! I think it makes sense to me what is going on here now.
|
👍 To this way of solving it. |
|
Also if anyone has any ideas to test any edge cases, I'm happy to add it. |
natefaubion
force-pushed
the
monadrec-sync-bounce
branch
from
859a596 to
b9fc891
Compare
|
I've pushed (what I think are) better inline comments, and also added a test that uses |
Avoids async bouncing by observing synchronous effects and looping.
natefaubion
force-pushed
the
monadrec-sync-bounce
branch
from
b9fc891 to
fe23499
Compare
|
I have added a test that verifies that a completely synchronous |
|
|
Nice work 
|
Great job figuring this out, @natefaubion! |
Ah, this helps explain a bug I was seeing, where forking an aff would still call the handler synchronously. Is this documented properly somewhere? Also, is there a way to force a true fork irrespective of whether the computation is synchronous or asynchronous. |
|
fork do
delay (Milliseconds 0.0)
.. |
The current implementation of
tailRecMis problematic because it will asynchronously bounce completely synchronous computations. This breaks the guarantee Aff provides that synchronous computations will always execute synchronously. In fact, one could argue that the current implementation breaks equational reasoning by introducing an observable difference between (essentially)leftEff do a; banddo liftEff a; liftEff b.This implementation is a bit hairier, but by observing synchronous effects in
tailRecM, we can guarantee that a totally synchronous computation remains synchronous, and that it is stack safe.This fixes SD-1665, and all tests pass on SlamData. Thoughts on this implementation?