Handler inheritance is now working

this code should also perform somewhat better

src/event/AsyncEvent.php

@@ -26,7 +26,6 @@ namespace pocketmine\event;
 use pocketmine\promise\Promise;
 use pocketmine\promise\PromiseResolver;
 use pocketmine\timings\Timings;
-use function array_shift;
 use function count;
 
 /**
@@ -61,7 +60,7 @@ abstract class AsyncEvent{
 			/** @phpstan-var PromiseResolver<static> $globalResolver */
 			$globalResolver = new PromiseResolver();
 
-			$this->asyncEachPriority(HandlerListManager::global()->getAsyncListFor(static::class), EventPriority::ALL, $globalResolver);
+			$this->processRemainingHandlers(HandlerListManager::global()->getAsyncHandlersFor(static::class), $globalResolver);
 
 			return $globalResolver->getPromise();
 		}finally{
@@ -71,84 +70,51 @@ abstract class AsyncEvent{
 	}
 
 	/**
-	 * TODO: this should use EventPriority constants for the list type but it's inconvenient with the current design
-	 * @phpstan-param list<int> $remaining
+	 * @param AsyncRegisteredListener[] $handlers
 	 * @phpstan-param PromiseResolver<static> $globalResolver
 	 */
-	private function asyncEachPriority(AsyncHandlerList $handlerList, array $remaining, PromiseResolver $globalResolver) : void{
-		while(true){
-			$nextPriority = array_shift($remaining);
-			if($nextPriority === null){
-				$globalResolver->resolve($this);
+	private function processRemainingHandlers(array $handlers, PromiseResolver $globalResolver) : void{
+		$currentPriority = null;
+		$awaitPromises = [];
+		foreach($handlers as $k => $handler){
+			$priority = $handler->getPriority();
+			if(count($awaitPromises) > 0 && $currentPriority !== null && $currentPriority !== $priority){
+				//wait for concurrent promises from previous priority to complete
 				break;
 			}
 
-			$promise = $this->callPriority($handlerList, $nextPriority);
-			if($promise !== null){
-				$promise->onCompletion(
-					onSuccess: fn() => $this->asyncEachPriority($handlerList, $remaining, $globalResolver),
-					onFailure: $globalResolver->reject(...)
-				);
-				break;
-			}
-		}
-	}
-
-	/**
-	 * @phpstan-return Promise<null>
-	 */
-	private function callPriority(AsyncHandlerList $handlerList, int $priority) : ?Promise{
-		$handlers = $handlerList->getListenersByPriority($priority);
-		if(count($handlers) === 0){
-			return null;
-		}
-
-		/** @phpstan-var PromiseResolver<null> $resolver */
-		$resolver = new PromiseResolver();
-
-		$concurrentPromises = [];
-		$nonConcurrentHandlers = [];
-		foreach($handlers as $registration){
-			if($registration->canBeCalledConcurrently()){
-				$result = $registration->callAsync($this);
-				if($result !== null) {
-					$concurrentPromises[] = $result;
+			$currentPriority = $priority;
+			if($handler->canBeCalledConcurrently()){
+				unset($handlers[$k]);
+				$promise = $handler->callAsync($this);
+				if($promise !== null){
+					$awaitPromises[] = $promise;
 				}
 			}else{
-				$nonConcurrentHandlers[] = $registration;
+				if(count($awaitPromises) > 0){
+					//wait for concurrent promises to complete
+					break;
+				}
+
+				unset($handlers[$k]);
+				$promise = $handler->callAsync($this);
+				if($promise !== null){
+					$promise->onCompletion(
+						onSuccess: fn() => $this->processRemainingHandlers($handlers, $globalResolver),
+						onFailure: $globalResolver->reject(...)
+					);
+					return;
+				}
 			}
 		}
 
-		Promise::all($concurrentPromises)->onCompletion(
-			onSuccess: fn() => $this->processExclusiveHandlers($nonConcurrentHandlers, $resolver),
-			onFailure: $resolver->reject(...)
-		);
-
-		return $resolver->getPromise();
-	}
-
-	/**
-	 * @param AsyncRegisteredListener[] $handlers
-	 * @phpstan-param PromiseResolver<null> $resolver
-	 */
-	private function processExclusiveHandlers(array $handlers, PromiseResolver $resolver) : void{
-		while(true){
-			$handler = array_shift($handlers);
-			if($handler === null){
-				$resolver->resolve(null);
-				break;
-			}
-			$result = $handler->callAsync($this);
-			if($result instanceof Promise){
-				//wait for this promise to resolve before calling the next handler
-				$result->onCompletion(
-					onSuccess: fn() => $this->processExclusiveHandlers($handlers, $resolver),
-					onFailure: $resolver->reject(...)
-				);
-				break;
-			}
-
-			//this handler didn't return a promise - continue directly to the next one
+		if(count($awaitPromises) > 0){
+			Promise::all($awaitPromises)->onCompletion(
+				onSuccess: fn() => $this->processRemainingHandlers($handlers, $globalResolver),
+				onFailure: $globalResolver->reject(...)
+			);
+		}else{
+			$globalResolver->resolve($this);
 		}
 	}
 }

src/event/AsyncHandlerList.php

@@ -24,25 +24,47 @@ declare(strict_types=1);
 namespace pocketmine\event;
 
 use pocketmine\plugin\Plugin;
+use function array_merge;
+use function krsort;
 use function spl_object_id;
+use function uasort;
+use const SORT_NUMERIC;
 
 class AsyncHandlerList{
+	//TODO: we can probably deduplicate most of this code with the sync side if we throw in some generics
+
 	/** @var AsyncRegisteredListener[][] */
 	private array $handlerSlots = [];
 
 	/**
-	 * @phpstan-param class-string<AsyncEvent> $class
+	 * @var RegisteredListenerCache[]
+	 * @phpstan-var array<int, RegisteredListenerCache<AsyncRegisteredListener>>
+	 */
+	private array $affectedHandlerCaches = [];
+
+	/**
+	 * @phpstan-param class-string<covariant AsyncEvent> $class
+	 * @phpstan-param RegisteredListenerCache<AsyncRegisteredListener> $handlerCache
 	 */
 	public function __construct(
 		private string $class,
 		private ?AsyncHandlerList $parentList,
-	){}
+		private RegisteredListenerCache $handlerCache = new RegisteredListenerCache()
+	){
+		for($list = $this; $list !== null; $list = $list->parentList){
+			$list->affectedHandlerCaches[spl_object_id($this->handlerCache)] = $this->handlerCache;
+		}
+	}
 
+	/**
+	 * @throws \Exception
+	 */
 	public function register(AsyncRegisteredListener $listener) : void{
 		if(isset($this->handlerSlots[$listener->getPriority()][spl_object_id($listener)])){
 			throw new \InvalidArgumentException("This listener is already registered to priority {$listener->getPriority()} of event {$this->class}");
 		}
 		$this->handlerSlots[$listener->getPriority()][spl_object_id($listener)] = $listener;
+		$this->invalidateAffectedCaches();
 	}
 
 	/**
@@ -52,10 +74,10 @@ class AsyncHandlerList{
 		foreach($listeners as $listener){
 			$this->register($listener);
 		}
+		$this->invalidateAffectedCaches();
 	}
 
 	public function unregister(AsyncRegisteredListener|Plugin|Listener $object) : void{
-		//TODO: Not loving the duplication here
 		if($object instanceof Plugin || $object instanceof Listener){
 			foreach($this->handlerSlots as $priority => $list){
 				foreach($list as $hash => $listener){
@@ -69,10 +91,12 @@ class AsyncHandlerList{
 		}else{
 			unset($this->handlerSlots[$object->getPriority()][spl_object_id($object)]);
 		}
+		$this->invalidateAffectedCaches();
 	}
 
 	public function clear() : void{
 		$this->handlerSlots = [];
+		$this->invalidateAffectedCaches();
 	}
 
 	/**
@@ -85,4 +109,48 @@ class AsyncHandlerList{
 	public function getParent() : ?AsyncHandlerList{
 		return $this->parentList;
 	}
+
+	/**
+	 * Invalidates all known caches which might be affected by this list's contents.
+	 */
+	private function invalidateAffectedCaches() : void{
+		foreach($this->affectedHandlerCaches as $cache){
+			$cache->list = null;
+		}
+	}
+
+	/**
+	 * @return AsyncRegisteredListener[]
+	 * @phpstan-return list<AsyncRegisteredListener>
+	 */
+	public function getListenerList() : array{
+		if($this->handlerCache->list !== null){
+			return $this->handlerCache->list;
+		}
+
+		$handlerLists = [];
+		for($currentList = $this; $currentList !== null; $currentList = $currentList->parentList){
+			$handlerLists[] = $currentList;
+		}
+
+		$listenersByPriority = [];
+		foreach($handlerLists as $currentList){
+			foreach($currentList->handlerSlots as $priority => $listeners){
+				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;
+				});
+				$listenersByPriority[$priority] = array_merge($listenersByPriority[$priority] ?? [], $listeners);
+			}
+		}
+
+		//TODO: why on earth do the priorities have higher values for lower priority?
+		krsort($listenersByPriority, SORT_NUMERIC);
+
+		return $this->handlerCache->list = array_merge(...$listenersByPriority);
+	}
 }

src/event/HandlerList.php

@@ -33,11 +33,15 @@ class HandlerList{
 	/** @var RegisteredListener[][] */
 	private array $handlerSlots = [];
 
-	/** @var RegisteredListenerCache[] */
+	/**
+	 * @var RegisteredListenerCache[]
+	 * @phpstan-var array<int, RegisteredListenerCache<RegisteredListener>>
+	 */
 	private array $affectedHandlerCaches = [];
 
 	/**
 	 * @phpstan-param class-string<covariant Event> $class
+	 * @phpstan-param RegisteredListenerCache<RegisteredListener> $handlerCache
 	 */
 	public function __construct(
 		private string $class,

src/event/HandlerListManager.php

@@ -38,12 +38,17 @@ class HandlerListManager{
 	private array $allSyncLists = [];
 	/**
 	 * @var RegisteredListenerCache[] event class name => cache
-	 * @phpstan-var array<class-string<Event|AsyncEvent>, RegisteredListenerCache>
+	 * @phpstan-var array<class-string<Event>, RegisteredListenerCache<RegisteredListener>>
 	 */
 	private array $syncHandlerCaches = [];
 
 	/** @var AsyncHandlerList[] classname => AsyncHandlerList */
 	private array $allAsyncLists = [];
+	/**
+	 * @var RegisteredListenerCache[] event class name => cache
+	 * @phpstan-var array<class-string<AsyncEvent>, RegisteredListenerCache<AsyncRegisteredListener>>
+	 */
+	private array $asyncHandlerCaches = [];
 
 	/**
 	 * Unregisters all the listeners
@@ -119,6 +124,7 @@ class HandlerListManager{
 		}
 
 		$parent = self::resolveNearestHandleableParent($class);
+		/** @phpstan-var RegisteredListenerCache<RegisteredListener> $cache */
 		$cache = new RegisteredListenerCache();
 		$this->syncHandlerCaches[$event] = $cache;
 		return $this->allSyncLists[$event] = new HandlerList(
@@ -150,9 +156,13 @@ class HandlerListManager{
 		}
 
 		$parent = self::resolveNearestHandleableParent($class);
+		/** @phpstan-var RegisteredListenerCache<AsyncRegisteredListener> $cache */
+		$cache = new RegisteredListenerCache();
+		$this->asyncHandlerCaches[$event] = $cache;
 		return $this->allAsyncLists[$event] = new AsyncHandlerList(
 			$event,
 			parentList: $parent !== null ? $this->getAsyncListFor($parent->getName()) : null,
+			handlerCache: $cache
 		);
 	}
 
@@ -167,6 +177,17 @@ class HandlerListManager{
 		return $cache?->list ?? $this->getListFor($event)->getListenerList();
 	}
 
+	/**
+	 * @phpstan-param class-string<covariant AsyncEvent> $event
+	 *
+	 * @return AsyncRegisteredListener[]
+	 */
+	public function getAsyncHandlersFor(string $event) : array{
+		$cache = $this->asyncHandlerCaches[$event] ?? null;
+		//getListFor() will populate the cache for the next call
+		return $cache?->list ?? $this->getAsyncListFor($event)->getListenerList();
+	}
+
 	/**
 	 * @return HandlerList[]
 	 */

src/event/RegisteredListenerCache.php

@@ -25,14 +25,14 @@ namespace pocketmine\event;
 
 /**
  * @internal
+ * @phpstan-template TListener
  */
 final class RegisteredListenerCache{
 
 	/**
 	 * List of all handlers that will be called for a particular event, ordered by execution order.
 	 *
-	 * @var RegisteredListener[]
-	 * @phpstan-var list<RegisteredListener>
+	 * @phpstan-var list<TListener>
 	 */
 	public ?array $list = null;
 }

tests/plugins/TesterPlugin/src/AsyncEventInheritanceTest.php

@@ -58,11 +58,12 @@ final class AsyncEventInheritanceTest extends Test{
 		$plugin = $this->getPlugin();
 		$classes = self::EXPECTED_ORDER;
 		shuffle($classes);
-		foreach($classes as $event){
+		foreach($classes as $class){
 			$plugin->getServer()->getPluginManager()->registerAsyncEvent(
-				$event,
-				function(AsyncEvent $event) : ?Promise{
-					$this->callOrder[] = $event::class;
+				$class,
+				function(AsyncEvent $event) use ($class) : ?Promise{
+					var_dump($class);
+					$this->callOrder[] = $class;
 					return null;
 				},
 				EventPriority::NORMAL,