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

Update functions.php #430

Closed
wants to merge 1 commit into from
Closed

Update functions.php #430

wants to merge 1 commit into from

Conversation

lav45
Copy link

@lav45 lav45 commented Feb 20, 2024

Skip delay if timeout is equels 0.0

Skip delay if timeout is equels 0.0
@bwoebi
Copy link
Member

bwoebi commented Feb 20, 2024

\Amp\delay(0); is an intentional feature to force the fiber to continue in the next event-loop tick.

So, no, we don't want to skip this. ... Especially not in 3.x, because it would be a breaking change :-/

Even though I agree, a separate function for "schedule in next tick" rather than \Amp\delay(0) would have been good, but too late now.

@bwoebi bwoebi closed this Feb 20, 2024
@trowski
Copy link
Member

trowski commented Feb 20, 2024

@bwoebi FYI, Amp\async(fn () => doSomething()) effectively will schedule something in the next tick. Amp\delay(0) is a nice way of pausing the current fiber for a tick, something which I generally only need to do in tests anyway.

@bwoebi
Copy link
Member

bwoebi commented Feb 20, 2024

@trowski async runs after all current outstanding microtasks, not next event-loop tick. awaitOutstandingMicrotasks() and awaitEventloopTick() would be sort of nice to have sometimes, explicitly.

@trowski
Copy link
Member

trowski commented Feb 20, 2024

Yes, though I find it rare to need to make such a distinction. I suppose if you're dealing with event-loop callback timing in a test that timing is going to matter. In that case, there's also EventLoop::defer(), which I guess is actually what I should have suggested over Amp\async().

@lav45 lav45 deleted the patch-1 branch February 20, 2024 23:01
@xpader
Copy link
Contributor

xpader commented Feb 21, 2024

I think there is no reason to change this.

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.

4 participants