Don't yield in async await if future is ready

[RedBeard0531]

This is one of my first macros and I just used the output from dumpAstGen. Let me know if there is a better way to write this.

Without the test change, it was printing 4 Done 5 Finished. I think that is because the call to await fs.write() no longer blocked progress of the alpha coroutine. That seems like a valid semantic to me, but let me know if there is some subtlety I'm missing.

[dom96]

There should be no need for this as the cb0 proc which handles the execution of async procedures already handles this logic. What does this fix?

[RedBeard0531]

It is substantially faster and avoids an unfortunate deep recursion in the current implementation.

import asyncdispatch

let done = newFuture[int]()
done.complete(1)

proc asyncSum: Future[int] {.async.} =
  for _ in 1..10_000_000:
    result += await done

echo waitFor asyncSum()

baseline devel: stack overflow from too much recursion
devel + @yglukhov's #6614 to avoid recursion: 592ms
devel + this patch: 50ms

This may be due to an unusual design, but in my current use case awaiting a finished future is actually the common case.

[Araq]

If you want if not fut.completed: yield fut why not write that? Maybe we can change the async macro to also support awaitPeek fut or something but the current behaviour should not be changed. It's already hard enough to figure out what is going on and you made it more undeterministic (most of the time I get lucky and avoid the await, kabooms my server in release mode---).

[dom96]

The fact that you needed to change the test makes me uneasy. No idea why that output would happen but it deserves more investigation, there might be a deeper problem with your patch.

[RedBeard0531]

The test change is because fs.write() always returns a ready future. Previously it would still cause alpha to yield to beta giving it a chance to print "5" before "Done". With my change alpha doesn't "block" on await fs.write(i) so it is able to print "Done" before beta can run. Moving the await sleepAsync(1000) call (which is never already ready) after the write() ensures that alpha gets a chance to run between beta writing 5 and finishing.

The semantics of only yielding on non-ready futures seems to match at least C#, Scala, and C++ implementations of async/await. It also matches the behavior of @zielmicha's reactor.nim async macro. I haven't yet figured out how python handles this case.

[dom96]

So as I mentioned, I would prefer to handle this in cb0. And it seems my patch runs into the same issue as yours :)

[dom96]

Closed by #6962