-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Generalize asynchronous events #6092
Open
ShockedPlot7560
wants to merge
56
commits into
minor-next
Choose a base branch
from
feat/async-events
base: minor-next
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 48 commits
Commits
Show all changes
56 commits
Select commit
Hold shift + click to select a range
5fe57a8
temporaly add Promise::all
ShockedPlot7560 a84fc2b
introduce AsyncEvent and ::callAsync()
ShockedPlot7560 7a4b9a0
events: asynchandler is defined by their return type and event type
ShockedPlot7560 9b2b92a
oops, remove test code
ShockedPlot7560 b78ff00
fix style
ShockedPlot7560 c250bb0
undo Promise covariant + improve array types
ShockedPlot7560 58155a7
fix PHPstan
ShockedPlot7560 2b2fa9d
phpstan: populate baseline
ShockedPlot7560 1176b70
Update src/player/Player.php
dktapps dc85bba
merge remote tracking
ShockedPlot7560 ed739cf
cannot call async event in sync context + remove Event dependency for…
ShockedPlot7560 7e87fbb
clarifying the exception message
ShockedPlot7560 5beaa3c
correction of various problems
ShockedPlot7560 cc6e8ef
move the asynchronous registration of handlers to a dedicated PluginM…
ShockedPlot7560 ca95b2f
fix PHPStan
ShockedPlot7560 823d4ea
inconsistency correction
ShockedPlot7560 243a303
follow up of #6110
ShockedPlot7560 aaa37ba
handlerListe: reduce code complexity
ShockedPlot7560 f82c422
remove using of Event API
ShockedPlot7560 64bbff6
Merge remote-tracking branch 'upstream/minor-next' into feat/async-ev…
ShockedPlot7560 d6b7a9e
merge remote tracking upstream
ShockedPlot7560 eb98141
resolve AsyncEvent with self instance
ShockedPlot7560 c1e3903
fix PHPstan
ShockedPlot7560 b276133
Merge remote-tracking branch 'origin/minor-next' into feat/async-events
ShockedPlot7560 86fb041
Merge branch 'minor-next' of github.com:pmmp/PocketMine-MP into feat/…
dktapps b82d47d
Merge branch 'minor-next' into feat/async-events
dktapps 48d2430
Update PHPStan baseline
dktapps 8f48fe4
Fully separate hierarchies for sync & async events
dktapps 17ae932
HandlerListManager: added getter
dktapps c426677
optimization
dktapps db88e54
Fix PHPStan error
dktapps a14afb4
Add integration tests
dktapps cb2fade
Fixed bug in concurrency integration test
dktapps 409066c
AsyncEvent: make the code easier to make sense of
dktapps a6a44bd
Fix doc comments
dktapps 6f40c6f
CS
dktapps 32b1d6c
Fixed test code
dktapps fa79653
ah hello my old friend, impossible-generics.neon
dktapps 8aed5d6
Handler inheritance is now working
dktapps 96989d1
cleanup
dktapps ac1cf73
Reduce code duplication
dktapps 972a9fb
PluginManager: ensure that handler candidates of async events with wr…
dktapps 667656b
Split AsyncHandlerListManager
dktapps edae9f2
Reduce number of classes
dktapps 11fdf79
...
dktapps 0a56cf8
Remove unused class
dktapps a7a1077
CONTRIBUTING: changing an event from sync to async or vice versa is a…
dktapps 117026c
Merge branch 'minor-next' into feat/async-events
dktapps d2d663b
Simplify handler sorting
dktapps 4451770
Merge branch 'minor-next' into feat/async-events
dktapps 406e2c6
Convert integration tests to unit tests
dktapps d9f5634
CS
dktapps d9080f1
we don't need a fake server instance outside of setUp() in these tests
dktapps 866d473
Merge branch 'minor-next' into feat/async-events
dktapps e8ec81d
fix PHPStan error
dktapps a0d69a9
github web editor don't fuck up indentation, challenge impossible
dktapps File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,126 @@ | ||
<?php | ||
|
||
/* | ||
* | ||
* ____ _ _ __ __ _ __ __ ____ | ||
* | _ \ ___ ___| | _____| |_| \/ (_)_ __ ___ | \/ | _ \ | ||
* | |_) / _ \ / __| |/ / _ \ __| |\/| | | '_ \ / _ \_____| |\/| | |_) | | ||
* | __/ (_) | (__| < __/ |_| | | | | | | | __/_____| | | | __/ | ||
* |_| \___/ \___|_|\_\___|\__|_| |_|_|_| |_|\___| |_| |_|_| | ||
* | ||
* This program is free software: you can redistribute it and/or modify | ||
* it under the terms of the GNU Lesser General Public License as published by | ||
* the Free Software Foundation, either version 3 of the License, or | ||
* (at your option) any later version. | ||
* | ||
* @author PocketMine Team | ||
* @link http://www.pocketmine.net/ | ||
* | ||
* | ||
*/ | ||
|
||
declare(strict_types=1); | ||
|
||
namespace pocketmine\event; | ||
|
||
use pocketmine\promise\Promise; | ||
use pocketmine\promise\PromiseResolver; | ||
use pocketmine\timings\Timings; | ||
use function count; | ||
|
||
/** | ||
* This class is used to permit asynchronous event handling. | ||
* | ||
* When an event is called asynchronously, the event handlers are called by priority level. | ||
* When all the promises of a priority level have been resolved, the next priority level is called. | ||
*/ | ||
abstract class AsyncEvent{ | ||
/** @var array<class-string<AsyncEvent>, int> $delegatesCallDepth */ | ||
private static array $delegatesCallDepth = []; | ||
private const MAX_EVENT_CALL_DEPTH = 50; | ||
|
||
/** | ||
* @phpstan-return Promise<static> | ||
*/ | ||
final public function call() : Promise{ | ||
if(!isset(self::$delegatesCallDepth[$class = static::class])){ | ||
self::$delegatesCallDepth[$class] = 0; | ||
} | ||
|
||
if(self::$delegatesCallDepth[$class] >= self::MAX_EVENT_CALL_DEPTH){ | ||
//this exception will be caught by the parent event call if all else fails | ||
throw new \RuntimeException("Recursive event call detected (reached max depth of " . self::MAX_EVENT_CALL_DEPTH . " calls)"); | ||
} | ||
|
||
$timings = Timings::getAsyncEventTimings($this); | ||
$timings->startTiming(); | ||
|
||
++self::$delegatesCallDepth[$class]; | ||
try{ | ||
/** @phpstan-var PromiseResolver<static> $globalResolver */ | ||
$globalResolver = new PromiseResolver(); | ||
|
||
$handlers = AsyncHandlerListManager::global()->getHandlersFor(static::class); | ||
if(count($handlers) > 0){ | ||
dktapps marked this conversation as resolved.
Show resolved
Hide resolved
|
||
$this->processRemainingHandlers($handlers, fn() => $globalResolver->resolve($this), $globalResolver->reject(...)); | ||
}else{ | ||
$globalResolver->resolve($this); | ||
} | ||
|
||
return $globalResolver->getPromise(); | ||
}finally{ | ||
--self::$delegatesCallDepth[$class]; | ||
$timings->stopTiming(); | ||
} | ||
} | ||
|
||
/** | ||
* @param AsyncRegisteredListener[] $handlers | ||
* @phpstan-param \Closure() : void $resolve | ||
* @phpstan-param \Closure() : void $reject | ||
*/ | ||
private function processRemainingHandlers(array $handlers, \Closure $resolve, \Closure $reject) : void{ | ||
$currentPriority = null; | ||
$awaitPromises = []; | ||
foreach($handlers as $k => $handler){ | ||
dktapps marked this conversation as resolved.
Show resolved
Hide resolved
|
||
$priority = $handler->getPriority(); | ||
if(count($awaitPromises) > 0 && $currentPriority !== null && $currentPriority !== $priority){ | ||
//wait for concurrent promises from previous priority to complete | ||
break; | ||
} | ||
|
||
$currentPriority = $priority; | ||
if($handler->canBeCalledConcurrently()){ | ||
unset($handlers[$k]); | ||
$promise = $handler->callAsync($this); | ||
if($promise !== null){ | ||
$awaitPromises[] = $promise; | ||
} | ||
}else{ | ||
if(count($awaitPromises) > 0){ | ||
//wait for concurrent promises to complete | ||
break; | ||
SOF3 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
unset($handlers[$k]); | ||
$promise = $handler->callAsync($this); | ||
if($promise !== null){ | ||
$promise->onCompletion( | ||
onSuccess: fn() => $this->processRemainingHandlers($handlers, $resolve, $reject), | ||
onFailure: $reject | ||
); | ||
return; | ||
} | ||
} | ||
} | ||
|
||
if(count($awaitPromises) > 0){ | ||
Promise::all($awaitPromises)->onCompletion( | ||
onSuccess: fn() => $this->processRemainingHandlers($handlers, $resolve, $reject), | ||
onFailure: $reject | ||
); | ||
}else{ | ||
$resolve(); | ||
} | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,61 @@ | ||
<?php | ||
|
||
/* | ||
* | ||
* ____ _ _ __ __ _ __ __ ____ | ||
* | _ \ ___ ___| | _____| |_| \/ (_)_ __ ___ | \/ | _ \ | ||
* | |_) / _ \ / __| |/ / _ \ __| |\/| | | '_ \ / _ \_____| |\/| | |_) | | ||
* | __/ (_) | (__| < __/ |_| | | | | | | | __/_____| | | | __/ | ||
* |_| \___/ \___|_|\_\___|\__|_| |_|_|_| |_|\___| |_| |_|_| | ||
* | ||
* This program is free software: you can redistribute it and/or modify | ||
* it under the terms of the GNU Lesser General Public License as published by | ||
* the Free Software Foundation, either version 3 of the License, or | ||
* (at your option) any later version. | ||
* | ||
* @author PocketMine Team | ||
* @link http://www.pocketmine.net/ | ||
* | ||
* | ||
*/ | ||
|
||
declare(strict_types=1); | ||
|
||
namespace pocketmine\event; | ||
|
||
use function uasort; | ||
|
||
/** | ||
* @phpstan-extends BaseHandlerListManager<AsyncEvent, AsyncRegisteredListener> | ||
*/ | ||
final class AsyncHandlerListManager extends BaseHandlerListManager{ | ||
private static ?self $globalInstance = null; | ||
|
||
public static function global() : self{ | ||
return self::$globalInstance ?? (self::$globalInstance = new self()); | ||
} | ||
|
||
protected function getBaseEventClass() : string{ | ||
return AsyncEvent::class; | ||
} | ||
|
||
/** | ||
* @phpstan-param array<int, AsyncRegisteredListener> $listeners | ||
* @phpstan-return array<int, AsyncRegisteredListener> | ||
*/ | ||
private static function sortSamePriorityHandlers(array $listeners) : array{ | ||
uasort($listeners, function(AsyncRegisteredListener $left, AsyncRegisteredListener $right) : int{ | ||
//While the system can handle these in any order, it's better for latency if concurrent handlers | ||
//are processed together. It doesn't matter whether they are processed before or after exclusive handlers. | ||
if($right->canBeCalledConcurrently()){ | ||
return $left->canBeCalledConcurrently() ? 0 : 1; | ||
} | ||
return -1; | ||
dktapps marked this conversation as resolved.
Show resolved
Hide resolved
|
||
}); | ||
return $listeners; | ||
} | ||
|
||
protected function createHandlerList(string $event, ?HandlerList $parentList, RegisteredListenerCache $handlerCache) : HandlerList{ | ||
return new HandlerList($event, $parentList, $handlerCache, self::sortSamePriorityHandlers(...)); | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,60 @@ | ||
<?php | ||
|
||
/* | ||
* | ||
* ____ _ _ __ __ _ __ __ ____ | ||
* | _ \ ___ ___| | _____| |_| \/ (_)_ __ ___ | \/ | _ \ | ||
* | |_) / _ \ / __| |/ / _ \ __| |\/| | | '_ \ / _ \_____| |\/| | |_) | | ||
* | __/ (_) | (__| < __/ |_| | | | | | | | __/_____| | | | __/ | ||
* |_| \___/ \___|_|\_\___|\__|_| |_|_|_| |_|\___| |_| |_|_| | ||
* | ||
* This program is free software: you can redistribute it and/or modify | ||
* it under the terms of the GNU Lesser General Public License as published by | ||
* the Free Software Foundation, either version 3 of the License, or | ||
* (at your option) any later version. | ||
* | ||
* @author PocketMine Team | ||
* @link http://www.pocketmine.net/ | ||
* | ||
* | ||
*/ | ||
|
||
declare(strict_types=1); | ||
|
||
namespace pocketmine\event; | ||
|
||
use pocketmine\plugin\Plugin; | ||
use pocketmine\promise\Promise; | ||
use pocketmine\timings\TimingsHandler; | ||
|
||
class AsyncRegisteredListener extends BaseRegisteredListener{ | ||
public function __construct( | ||
\Closure $handler, | ||
int $priority, | ||
Plugin $plugin, | ||
bool $handleCancelled, | ||
private bool $exclusiveCall, | ||
TimingsHandler $timings | ||
){ | ||
parent::__construct($handler, $priority, $plugin, $handleCancelled, $timings); | ||
} | ||
|
||
/** | ||
* @phpstan-return Promise<null>|null | ||
*/ | ||
public function callAsync(AsyncEvent $event) : ?Promise{ | ||
if($event instanceof Cancellable && $event->isCancelled() && !$this->isHandlingCancelled()){ | ||
return null; | ||
} | ||
$this->timings->startTiming(); | ||
try{ | ||
return ($this->handler)($event); | ||
}finally{ | ||
$this->timings->stopTiming(); | ||
} | ||
} | ||
|
||
public function canBeCalledConcurrently() : bool{ | ||
return !$this->exclusiveCall; | ||
} | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if I dispatch 50 AsyncPlayerLoginEvents within 1 second when server starts, and the handlers take more than 1 second to resolve? The server crashes immediately?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I discussed this with Plot on Discord yesterday. I don't think even he knows how this code was supposed to work.