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

Improve await() to avoid unneeded futureTick() calls #32

Merged
merged 4 commits into from
Feb 18, 2022

Conversation

clue
Copy link
Member

@clue clue commented Feb 18, 2022

This changeset improves the await() function to avoid any unneeded futureTick() calls. This means we can now rely on its execution behavior just like all other promise functions and the coroutine() function (#12) provided by this package.

The await() function is used to await the resolution of a promise in fiber-based asynchronous programs. Once the promise is fulfilled (or rejected), execution will now continue immediately. This is also in line with how the coroutine() function continues execution immediately after a yield statement.

In the previous version, this would have to await the next "loop tick" after the resolution which would be both unexpected from a consumer's perspective and slower in execution. The unexpected behavior has caused a number of issues in the past, most recently #27 which essentially boils down to multiple events happening in the same "tick" which is now supported just fine.

The test suite confirms this only adds new features not previously possible with the old approach and does not otherwise affect existing behavior. Affected code paths have 100% code coverage (I'll look into ensuring we have full code coverage also for unrelated code paths in a separate PR).

This changeset also promises some noticeable performance improvements in synthetic benchmarks, see also #31 for more details about benchmarks. Together with #30, I've seen some 25% performance improvements also inside Framework X for some use cases, but my main motivation is really avoiding unexpected behavior and solving related issues, so I do not expect a significant performance effect for most real-world applications.

I've kept my original commits and a final refactoring in place to ease reviewing and to show how I've gone about removing each group of ticks. The resulting code looks surprisingly straight-forward, but you're look at 8+ hours of work on this PR alone. Enjoy!

@clue clue added the new feature New feature or request label Feb 18, 2022
@clue clue added this to the v4.0.0 milestone Feb 18, 2022
@WyriHaximus WyriHaximus merged commit 4cadacc into reactphp:4.x Feb 18, 2022
@clue clue deleted the the-future-is-now branch February 19, 2022 09:36
WyriHaximus added a commit to WyriHaximus-labs/async that referenced this pull request Feb 20, 2022
Since `async()` returns a promise and those are normally cancelable, implementing this puts them in line with the rest of our ecosystem. As such the following example will throw a timeout exception from the canceled `sleep()` call.

```php
$promise = async(static function (): int {
    echo 'a';
    await(sleep(2));
    echo 'b';

    return time();
})();

$promise->cancel();
await($promise);
````

This builds on top of reactphp#15, reactphp#18, reactphp#19, reactphp#26, reactphp#28, reactphp#30, and reactphp#32.
WyriHaximus added a commit to WyriHaximus-labs/async that referenced this pull request Feb 21, 2022
Since `async()` returns a promise and those are normally cancelable, implementing this puts them in line with the rest of our ecosystem. As such the following example will throw a timeout exception from the canceled `sleep()` call.

```php
$promise = async(static function (): int {
    echo 'a';
    await(sleep(2));
    echo 'b';

    return time();
})();

$promise->cancel();
await($promise);
````

This builds on top of reactphp#15, reactphp#18, reactphp#19, reactphp#26, reactphp#28, reactphp#30, and reactphp#32.
WyriHaximus added a commit to WyriHaximus-labs/async that referenced this pull request Feb 21, 2022
Since `async()` returns a promise and those are normally cancelable, implementing this puts them in line with the rest of our ecosystem. As such the following example will throw a timeout exception from the canceled `sleep()` call.

```php
$promise = async(static function (): int {
    echo 'a';
    await(sleep(2));
    echo 'b';

    return time();
})();

$promise->cancel();
await($promise);
````

This builds on top of reactphp#15, reactphp#18, reactphp#19, reactphp#26, reactphp#28, reactphp#30, and reactphp#32.
WyriHaximus added a commit to WyriHaximus-labs/async that referenced this pull request Feb 24, 2022
Since `async()` returns a promise and those are normally cancelable, implementing this puts them in line with the rest of our ecosystem. As such the following example will throw a timeout exception from the canceled `sleep()` call.

```php
$promise = async(static function (): int {
    echo 'a';
    await(sleep(2));
    echo 'b';

    return time();
})();

$promise->cancel();
await($promise);
````

This builds on top of reactphp#15, reactphp#18, reactphp#19, reactphp#26, reactphp#28, reactphp#30, and reactphp#32.
WyriHaximus added a commit to WyriHaximus-labs/async that referenced this pull request Feb 24, 2022
Since `async()` returns a promise and those are normally cancelable, implementing this puts them in line with the rest of our ecosystem. As such the following example will throw a timeout exception from the canceled `sleep()` call.

```php
$promise = async(static function (): int {
    echo 'a';
    await(sleep(2));
    echo 'b';

    return time();
})();

$promise->cancel();
await($promise);
````

This builds on top of reactphp#15, reactphp#18, reactphp#19, reactphp#26, reactphp#28, reactphp#30, and reactphp#32.
WyriHaximus added a commit to WyriHaximus-labs/async that referenced this pull request Mar 2, 2022
Since `async()` returns a promise and those are normally cancelable, implementing this puts them in line with the rest of our ecosystem. As such the following example will throw a timeout exception from the canceled `sleep()` call.

```php
$promise = async(static function (): int {
    echo 'a';
    await(sleep(2));
    echo 'b';

    return time();
})();

$promise->cancel();
await($promise);
````

This builds on top of reactphp#15, reactphp#18, reactphp#19, reactphp#26, reactphp#28, reactphp#30, and reactphp#32.
WyriHaximus added a commit to WyriHaximus-labs/async that referenced this pull request Mar 3, 2022
Since `async()` returns a promise and those are normally cancelable, implementing this puts them in line with the rest of our ecosystem. As such the following example will throw a timeout exception from the canceled `sleep()` call.

```php
$promise = async(static function (): int {
    echo 'a';
    await(sleep(2));
    echo 'b';

    return time();
})();

$promise->cancel();
await($promise);
````

This builds on top of reactphp#15, reactphp#18, reactphp#19, reactphp#26, reactphp#28, reactphp#30, and reactphp#32.
WyriHaximus added a commit to WyriHaximus-labs/async that referenced this pull request Mar 3, 2022
Since `async()` returns a promise and those are normally cancelable, implementing this puts them in line with the rest of our ecosystem. As such the following example will throw a timeout exception from the canceled `sleep()` call.

```php
$promise = async(static function (): int {
    echo 'a';
    await(sleep(2));
    echo 'b';

    return time();
})();

$promise->cancel();
await($promise);
````

This builds on top of reactphp#15, reactphp#18, reactphp#19, reactphp#26, reactphp#28, reactphp#30, and reactphp#32.
WyriHaximus added a commit to WyriHaximus-labs/async that referenced this pull request Mar 4, 2022
Since `async()` returns a promise and those are normally cancelable, implementing this puts them in line with the rest of our ecosystem. As such the following example will throw a timeout exception from the canceled `sleep()` call.

```php
$promise = async(static function (): int {
    echo 'a';
    await(sleep(2));
    echo 'b';

    return time();
})();

$promise->cancel();
await($promise);
````

This builds on top of reactphp#15, reactphp#18, reactphp#19, reactphp#26, reactphp#28, reactphp#30, and reactphp#32.
WyriHaximus added a commit to WyriHaximus-labs/async that referenced this pull request Mar 4, 2022
Since `async()` returns a promise and those are normally cancelable, implementing this puts them in line with the rest of our ecosystem. As such the following example will throw a timeout exception from the canceled `sleep()` call.

```php
$promise = async(static function (): int {
    echo 'a';
    await(sleep(2));
    echo 'b';

    return time();
})();

$promise->cancel();
await($promise);
````

This builds on top of reactphp#15, reactphp#18, reactphp#19, reactphp#26, reactphp#28, reactphp#30, and reactphp#32.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

await() doesn't work with streaming HTTP response body
3 participants