SOF3 / await-generator

Write code in async/await style in PHP using generators.
https://sof3.github.io/await-generator/master/
Apache License 2.0
123 stars 15 forks source link

feat: allow cancellable race through throwing exceptions #194

Closed SOF3 closed 1 year ago

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage: 90.32% and project coverage change: -6.64 :warning:

Comparison is base (6cb96c7) 99.45% compared to head (16f0f49) 92.82%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #194 +/- ## ============================================ - Coverage 99.45% 92.82% -6.64% - Complexity 129 147 +18 ============================================ Files 10 11 +1 Lines 367 418 +51 ============================================ + Hits 365 388 +23 - Misses 2 30 +28 ``` | [Impacted Files](https://codecov.io/gh/SOF3/await-generator/pull/194?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jonathan+Chan+Kwan+Yin) | Coverage Δ | | |---|---|---| | [...enerator/src/SOFe/AwaitGenerator/GeneratorUtil.php](https://codecov.io/gh/SOF3/await-generator/pull/194?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jonathan+Chan+Kwan+Yin#diff-YXdhaXQtZ2VuZXJhdG9yL3NyYy9TT0ZlL0F3YWl0R2VuZXJhdG9yL0dlbmVyYXRvclV0aWwucGhw) | `57.14% <0.00%> (-42.86%)` | :arrow_down: | | [await-generator/src/SOFe/AwaitGenerator/Await.php](https://codecov.io/gh/SOF3/await-generator/pull/194?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jonathan+Chan+Kwan+Yin#diff-YXdhaXQtZ2VuZXJhdG9yL3NyYy9TT0ZlL0F3YWl0R2VuZXJhdG9yL0F3YWl0LnBocA==) | `89.87% <91.42%> (-10.13%)` | :arrow_down: | | [...wait-generator/src/SOFe/AwaitGenerator/Channel.php](https://codecov.io/gh/SOF3/await-generator/pull/194?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jonathan+Chan+Kwan+Yin#diff-YXdhaXQtZ2VuZXJhdG9yL3NyYy9TT0ZlL0F3YWl0R2VuZXJhdG9yL0NoYW5uZWwucGhw) | `100.00% <100.00%> (ø)` | | | [...wait-generator/src/SOFe/AwaitGenerator/Loading.php](https://codecov.io/gh/SOF3/await-generator/pull/194?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jonathan+Chan+Kwan+Yin#diff-YXdhaXQtZ2VuZXJhdG9yL3NyYy9TT0ZlL0F3YWl0R2VuZXJhdG9yL0xvYWRpbmcucGhw) | `95.45% <100.00%> (+1.01%)` | :arrow_up: | | [...ator/src/SOFe/AwaitGenerator/RaceLostException.php](https://codecov.io/gh/SOF3/await-generator/pull/194?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jonathan+Chan+Kwan+Yin#diff-YXdhaXQtZ2VuZXJhdG9yL3NyYy9TT0ZlL0F3YWl0R2VuZXJhdG9yL1JhY2VMb3N0RXhjZXB0aW9uLnBocA==) | `100.00% <100.00%> (ø)` | | | [...it-generator/src/SOFe/AwaitGenerator/Traverser.php](https://codecov.io/gh/SOF3/await-generator/pull/194?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jonathan+Chan+Kwan+Yin#diff-YXdhaXQtZ2VuZXJhdG9yL3NyYy9TT0ZlL0F3YWl0R2VuZXJhdG9yL1RyYXZlcnNlci5waHA=) | `100.00% <100.00%> (ø)` | | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jonathan+Chan+Kwan+Yin). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jonathan+Chan+Kwan+Yin)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

Endermanbugzjfc commented 1 year ago

Btw, have you ever thought to implement Channel's cancellation? I do not have a clear clue as race() fires all generators even though the first one resolves immediately.

SOF3 commented 1 year ago

Btw, have you ever thought to implement Channel's cancellation? I do not have a clear clue as race() fires all generators even though the first one resolves immediately.

What about we don't execute the subsequent generators at all if the first one returns immediately?

SOF3 commented 1 year ago

Since everything happens on the same thread, there must be a generator that resolves before others as long as we don't try to execute them and give them the chance to resolve.

Endermanbugzjfc commented 1 year ago

Btw, have you ever thought to implement Channel's cancellation? I do not have a clear clue as race() fires all generators even though the first one resolves immediately.

What about we don't execute the subsequent generators at all if the first one returns immediately?

But that would be a breaking change for race, should we not keep the breaking range inside safeRace?

SOF3 commented 1 year ago

Btw, have you ever thought to implement Channel's cancellation? I do not have a clear clue as race() fires all generators even though the first one resolves immediately.

What about we don't execute the subsequent generators at all if the first one returns immediately?

But that would be a breaking change for race, should we not keep the breaking range inside safeRace?

Right, I don't want to change the internals of the core generator-winding part because await-generator guarantees cross-shading compatibility — you can safely call {older version of await-generator}\Await::g2c() with generators that call {newer version of await-generator}\Await::race (or any other await-generator tools). The core runtime behind Await::g2c is almost frozen at this point to ensure cross-shading compatibility.

Maybe we can add a layer of wrapper in front of the generators in safeRace to support this. So safeRace wraps each generator like this:

race($inputs) {
    $channel = new Channel;
    $wrapped = [];
    foreach($inputs as $input) {
        $wrapped[] = (function() use($input, $channel) {
            yield from $channel->receive();
            $channel->sendWithoutWait(null);
            return yield from $input;
        })();
    }
    return (yield from Await::all([
        $wrapped,
        $channel->sendAndWait(null),
    ]))[0];
}

Since the generators never started, there is nothing for them to clean up.

SOF3 commented 1 year ago

That said, I am not very sure if this solution is clean enough... because this is a circular dependency between Channel and Await::race logic. There is no circular calling, but now the logic is quite coupled together...

SOF3 commented 1 year ago

Hmm, the solution above doesn't work, I need to mess with the yield from $input; part a bit more and call sendWithoutWait if and only if yield from $input; suspends. Ok this logic is completely cursed...

SOF3 commented 1 year ago

I think the comments above actually reveals a bug — safeRace is only safe for generators that do not resolve immediately.

SOF3 commented 1 year ago

I added some logic to throttle generator startup, but I am not exactly confident this works in all cases. Please help check if I missed any important test cases that might not fail.

SOF3 commented 1 year ago

Alright, the unit tests already revealed... I will check again tmr, it's a bit late now.