From 5d7feaaf2118c4474945ca6ccf90f594d6ff5510 Mon Sep 17 00:00:00 2001 From: Dylan T Date: Tue, 13 Nov 2018 21:04:47 +0000 Subject: [PATCH] Remove EventExecutor, event handlers now use closures (#2525) This cleans up some cargo-cult code poorly copied from Bukkit, which has negative performance effects and also makes internal event handling more complex than necessary. ## API changes - Removed `EventExecutor` and `MethodEventExecutor`. - A listener is no longer required for an event handler to be registered. Closure objects can now be used directly provided that they meet the conditions for registration. - `PluginManager->registerEvent()` signature has changed: the `Listener` and `EventExecutor` parameters have been removed and a `\Closure $handler` has been added in its place. - `RegisteredListener` now requires a `Closure` parameter instead of `Listener, EventExecutor`. ## Behavioural changes These changes reduce the execution complexity involved with calling an event handler. Since event calls can happen in hot paths, this may have visible positive effects on performance. Initial testing reveals a performance improvement of ~15% per event handler call compared to the old method. --- src/pocketmine/event/HandlerList.php | 2 +- src/pocketmine/plugin/EventExecutor.php | 38 ---------------- src/pocketmine/plugin/MethodEventExecutor.php | 44 ------------------- src/pocketmine/plugin/PluginManager.php | 27 ++++++------ src/pocketmine/plugin/RegisteredListener.php | 25 ++++------- 5 files changed, 23 insertions(+), 113 deletions(-) delete mode 100644 src/pocketmine/plugin/EventExecutor.php delete mode 100644 src/pocketmine/plugin/MethodEventExecutor.php diff --git a/src/pocketmine/event/HandlerList.php b/src/pocketmine/event/HandlerList.php index 9b2778705..833f0bb2a 100644 --- a/src/pocketmine/event/HandlerList.php +++ b/src/pocketmine/event/HandlerList.php @@ -140,7 +140,7 @@ class HandlerList{ foreach($this->handlerSlots as $priority => $list){ foreach($list as $hash => $listener){ if(($object instanceof Plugin and $listener->getPlugin() === $object) - or ($object instanceof Listener and $listener->getListener() === $object) + or ($object instanceof Listener and (new \ReflectionFunction($listener->getHandler()))->getClosureThis() === $object) //this doesn't even need to be a listener :D ){ unset($this->handlerSlots[$priority][$hash]); } diff --git a/src/pocketmine/plugin/EventExecutor.php b/src/pocketmine/plugin/EventExecutor.php deleted file mode 100644 index f7697a300..000000000 --- a/src/pocketmine/plugin/EventExecutor.php +++ /dev/null @@ -1,38 +0,0 @@ -method = $method; - } - - public function execute(Listener $listener, Event $event){ - $listener->{$this->getMethod()}($event); - } - - public function getMethod(){ - return $this->method; - } -} diff --git a/src/pocketmine/plugin/PluginManager.php b/src/pocketmine/plugin/PluginManager.php index 767bc7134..66a8dbce2 100644 --- a/src/pocketmine/plugin/PluginManager.php +++ b/src/pocketmine/plugin/PluginManager.php @@ -519,43 +519,44 @@ class PluginManager{ } } - $this->registerEvent($eventClass->getName(), $listener, $priority, new MethodEventExecutor($method->getName()), $plugin, $ignoreCancelled); + $this->registerEvent($eventClass->getName(), $handlerClosure, $priority, $plugin, $ignoreCancelled); } } } /** - * @param string $event Class name that extends Event - * @param Listener $listener - * @param int $priority - * @param EventExecutor $executor - * @param Plugin $plugin - * @param bool $ignoreCancelled + * @param string $event Class name that extends Event + * @param \Closure $handler + * @param int $priority + * @param Plugin $plugin + * @param bool $ignoreCancelled * - * @throws PluginException + * @throws \ReflectionException */ - public function registerEvent(string $event, Listener $listener, int $priority, EventExecutor $executor, Plugin $plugin, bool $ignoreCancelled = false) : void{ + public function registerEvent(string $event, \Closure $handler, int $priority, Plugin $plugin, bool $ignoreCancelled = false) : void{ if(!is_subclass_of($event, Event::class)){ throw new PluginException($event . " is not an Event"); } + $handlerName = Utils::getNiceClosureName($handler); + $tags = Utils::parseDocComment((string) (new \ReflectionClass($event))->getDocComment()); if(isset($tags["deprecated"]) and $this->server->getProperty("settings.deprecated-verbose", true)){ $this->server->getLogger()->warning($this->server->getLanguage()->translateString("pocketmine.plugin.deprecatedEvent", [ $plugin->getName(), $event, - get_class($listener) . "->" . ($executor instanceof MethodEventExecutor ? $executor->getMethod() : "") + $handlerName ])); } if(!$plugin->isEnabled()){ - throw new PluginException("Plugin attempted to register " . $event . " while not enabled"); + throw new PluginException("Plugin attempted to register event handler " . $handlerName . "() to event " . $event . " while not enabled"); } - $timings = new TimingsHandler("Plugin: " . $plugin->getDescription()->getFullName() . " Event: " . get_class($listener) . "::" . ($executor instanceof MethodEventExecutor ? $executor->getMethod() : "???") . "(" . (new \ReflectionClass($event))->getShortName() . ")"); + $timings = new TimingsHandler("Plugin: " . $plugin->getDescription()->getFullName() . " Event: " . $handlerName . "(" . (new \ReflectionClass($event))->getShortName() . ")"); - $this->getEventListeners($event)->register(new RegisteredListener($listener, $executor, $priority, $plugin, $ignoreCancelled, $timings)); + $this->getEventListeners($event)->register(new RegisteredListener($handler, $priority, $plugin, $ignoreCancelled, $timings)); } /** diff --git a/src/pocketmine/plugin/RegisteredListener.php b/src/pocketmine/plugin/RegisteredListener.php index 7db4cadea..0af6b197c 100644 --- a/src/pocketmine/plugin/RegisteredListener.php +++ b/src/pocketmine/plugin/RegisteredListener.php @@ -25,13 +25,12 @@ namespace pocketmine\plugin; use pocketmine\event\Cancellable; use pocketmine\event\Event; -use pocketmine\event\Listener; use pocketmine\timings\TimingsHandler; class RegisteredListener{ - /** @var Listener */ - private $listener; + /** @var \Closure */ + private $handler; /** @var int */ private $priority; @@ -39,9 +38,6 @@ class RegisteredListener{ /** @var Plugin */ private $plugin; - /** @var EventExecutor */ - private $executor; - /** @var bool */ private $ignoreCancelled; @@ -50,27 +46,22 @@ class RegisteredListener{ /** - * @param Listener $listener - * @param EventExecutor $executor + * @param \Closure $handler * @param int $priority * @param Plugin $plugin * @param bool $ignoreCancelled * @param TimingsHandler $timings */ - public function __construct(Listener $listener, EventExecutor $executor, int $priority, Plugin $plugin, bool $ignoreCancelled, TimingsHandler $timings){ - $this->listener = $listener; + public function __construct(\Closure $handler, int $priority, Plugin $plugin, bool $ignoreCancelled, TimingsHandler $timings){ + $this->handler = $handler; $this->priority = $priority; $this->plugin = $plugin; - $this->executor = $executor; $this->ignoreCancelled = $ignoreCancelled; $this->timings = $timings; } - /** - * @return Listener - */ - public function getListener() : Listener{ - return $this->listener; + public function getHandler() : \Closure{ + return $this->handler; } /** @@ -95,7 +86,7 @@ class RegisteredListener{ return; } $this->timings->startTiming(); - $this->executor->execute($this->listener, $event); + ($this->handler)($event); $this->timings->stopTiming(); }