From 3834843f842f617b6dfa5e551d3a252f2e0dd0b1 Mon Sep 17 00:00:00 2001 From: Marc Bennewitz Date: Sat, 5 Dec 2015 23:37:16 +0100 Subject: [PATCH 1/5] Performance increasement by integer priority --- src/EventManager.php | 20 +++++++++++------- src/SharedEventManager.php | 36 ++++++++++++++++++++------------- test/SharedEventManagerTest.php | 4 ++-- 3 files changed, 37 insertions(+), 23 deletions(-) diff --git a/src/EventManager.php b/src/EventManager.php index b41df06..717a9c6 100644 --- a/src/EventManager.php +++ b/src/EventManager.php @@ -164,7 +164,7 @@ public function attach($eventName, callable $listener, $priority = 1) )); } - $this->events[$eventName][((int) $priority) . '.0'][] = $listener; + $this->events[$eventName][(int) $priority][] = $listener; return $listener; } @@ -296,13 +296,19 @@ protected function triggerListeners(EventInterface $event, callable $callback = */ private function getListenersByEventName($eventName) { - $listeners = array_merge_recursive( - isset($this->events[$eventName]) ? $this->events[$eventName] : [], - isset($this->events['*']) ? $this->events['*'] : [], - $this->sharedManager ? $this->sharedManager->getListeners($this->identifiers, $eventName) : [] - ); + $listeners = isset($this->events[$eventName]) ? $this->events[$eventName] : []; + if (isset($this->events['*'])) { + foreach ($this->events['*'] as $p => $l) { + $listeners[$p] = isset($listeners[$p]) ? array_merge($listeners[$p], $l) : $l; + } + } + if ($this->sharedManager) { + foreach ($this->sharedManager->getListeners($this->identifiers, $eventName) as $p => $l) { + $listeners[$p] = isset($listeners[$p]) ? array_merge($listeners[$p], $l) : $l; + } + } - krsort($listeners, SORT_NUMERIC); + krsort($listeners); $listenersForEvent = []; diff --git a/src/SharedEventManager.php b/src/SharedEventManager.php index 750fee3..2b546b7 100644 --- a/src/SharedEventManager.php +++ b/src/SharedEventManager.php @@ -74,7 +74,7 @@ public function attach($identifier, $event, callable $listener, $priority = 1) )); } - $this->identifiers[$identifier][$event][((int) $priority) . '.0'][] = $listener; + $this->identifiers[$identifier][$event][(int) $priority][] = $listener; } /** @@ -178,22 +178,30 @@ public function getListeners(array $identifiers, $eventName) } $listenersByIdentifier = isset($this->identifiers[$identifier]) ? $this->identifiers[$identifier] : []; - - $listeners = array_merge_recursive( - $listeners, - isset($listenersByIdentifier[$eventName]) ? $listenersByIdentifier[$eventName] : [], - isset($listenersByIdentifier['*']) ? $listenersByIdentifier['*'] : [] - ); + if (isset($listenersByIdentifier[$eventName])) { + foreach ($listenersByIdentifier[$eventName] as $p => $l) { + $listeners[$p] = isset($listeners[$p]) ? array_merge($listeners[$p], $l) : $l; + } + } + if (isset($listenersByIdentifier['*'])) { + foreach ($listenersByIdentifier['*'] as $p => $l) { + $listeners[$p] = isset($listeners[$p]) ? array_merge($listeners[$p], $l) : $l; + } + } } - if (isset($this->identifiers['*']) && ! in_array('*', $identifiers)) { + if (isset($this->identifiers['*']) && ! in_array('*', $identifiers, true)) { $wildcardIdentifier = $this->identifiers['*']; - - $listeners = array_merge_recursive( - $listeners, - isset($wildcardIdentifier[$eventName]) ? $wildcardIdentifier[$eventName] : [], - isset($wildcardIdentifier['*']) ? $wildcardIdentifier['*'] : [] - ); + if (isset($wildcardIdentifier[$eventName])) { + foreach ($wildcardIdentifier[$eventName] as $p => $l) { + $listeners[$p] = isset($listeners[$p]) ? array_merge($listeners[$p], $l) : $l; + } + } + if (isset($wildcardIdentifier['*'])) { + foreach ($wildcardIdentifier['*'] as $p => $l) { + $listeners[$p] = isset($listeners[$p]) ? array_merge($listeners[$p], $l) : $l; + } + } } return $listeners; diff --git a/test/SharedEventManagerTest.php b/test/SharedEventManagerTest.php index 5d7e95a..cd3c1c7 100644 --- a/test/SharedEventManagerTest.php +++ b/test/SharedEventManagerTest.php @@ -27,7 +27,7 @@ public function setUp() public function getListeners(SharedEventManager $manager, array $identifiers, $event, $priority = 1) { - $priority = (int) $priority . '.0'; + $priority = (int) $priority; $listeners = $manager->getListeners($identifiers, $event); if (! isset($listeners[$priority])) { return []; @@ -247,7 +247,7 @@ public function testClearListenersDoesNothingIfNoEventsRegisteredForIdentifier() $this->manager->clearListeners('IDENTIFIER', 'EVENT'); // getListeners() always pulls in wildcard listeners - $this->assertEquals(['1.0' => [ + $this->assertEquals([1 => [ $this->callback, ]], $this->manager->getListeners([ 'IDENTIFIER' ], 'EVENT')); } From 263a6eb4bdc291f6dd2d9f3b76e50073412a0002 Mon Sep 17 00:00:00 2001 From: Marc Bennewitz Date: Fri, 11 Dec 2015 10:39:10 +0100 Subject: [PATCH 2/5] Performance optimization by reusing list of listeners instead of merging into a new array --- src/EventManager.php | 109 ++++++++++++++++++++----------------- src/SharedEventManager.php | 2 +- test/EventManagerTest.php | 7 ++- 3 files changed, 66 insertions(+), 52 deletions(-) diff --git a/src/EventManager.php b/src/EventManager.php index 717a9c6..7e24d96 100644 --- a/src/EventManager.php +++ b/src/EventManager.php @@ -164,7 +164,9 @@ public function attach($eventName, callable $listener, $priority = 1) )); } - $this->events[$eventName][(int) $priority][] = $listener; + // see performance note of triggerListeners() why the internal + // event array is structured this way. + $this->events[$eventName][(int) $priority][0][] = $listener; return $listener; } @@ -197,16 +199,16 @@ public function detach(callable $listener, $eventName = null, $force = false) } foreach ($this->events[$eventName] as $priority => $listeners) { - foreach ($listeners as $index => $evaluatedListener) { + foreach ($listeners[0] as $index => $evaluatedListener) { if ($evaluatedListener !== $listener) { continue; } // Found the listener; remove it. - unset($this->events[$eventName][$priority][$index]); + unset($this->events[$eventName][$priority][0][$index]); // If the queue for the given priority is empty, remove it. - if (empty($this->events[$eventName][$priority])) { + if (empty($this->events[$eventName][$priority][0])) { unset($this->events[$eventName][$priority]); break; } @@ -262,64 +264,71 @@ protected function triggerListeners(EventInterface $event, callable $callback = throw new Exception\RuntimeException('Event is missing a name; cannot trigger!'); } - // Initial value of stop propagation flag should be false - $event->stopPropagation(false); - - $responses = new ResponseCollection(); - - foreach ($this->getListenersByEventName($name) as $listener) { - $response = $listener($event); - $responses->push($response); - - // If the event was asked to stop propagating, do so - if ($event->propagationIsStopped()) { - $responses->setStopped(true); - break; - } - - // If the result causes our validation callback to return true, - // stop propagation - if ($callback && $callback($response)) { - $responses->setStopped(true); - break; - } - } - - return $responses; - } - - /** - * Get listeners for the currently triggered event. - * - * @param string $eventName - * @return callable[] - */ - private function getListenersByEventName($eventName) - { - $listeners = isset($this->events[$eventName]) ? $this->events[$eventName] : []; + // Merge all listeners by priority together with format + // [ + // => [ + // [ + // [, , ...] + // ], + // ... + // ], + // => [ + // [ + // [, , ...] + // ], + // ... + // ], + // ... + // ] + // + // PERFORMANCE NOTE: + // This way pushing the list of listeners into a new generated list + // helps us to not call "array_merge_[recursive]" + // and reusing the list of listeners we already have + // instead of iterating over it and generating a new one. + // -> In result it improves performance by up to 25% even if it looks a bit strange. + $listOfListenersByPriority = isset($this->events[$name]) ? $this->events[$name] : []; if (isset($this->events['*'])) { foreach ($this->events['*'] as $p => $l) { - $listeners[$p] = isset($listeners[$p]) ? array_merge($listeners[$p], $l) : $l; + $listOfListenersByPriority[$p][] = $l[0]; } } if ($this->sharedManager) { - foreach ($this->sharedManager->getListeners($this->identifiers, $eventName) as $p => $l) { - $listeners[$p] = isset($listeners[$p]) ? array_merge($listeners[$p], $l) : $l; + foreach ($this->sharedManager->getListeners($this->identifiers, $name) as $p => $l) { + $listOfListenersByPriority[$p][] = $l; } } - krsort($listeners); + // Sort by priority in reverse order + krsort($listOfListenersByPriority); - $listenersForEvent = []; + // Initial value of stop propagation flag should be false + $event->stopPropagation(false); - foreach ($listeners as $priority => $listenersByPriority) { - foreach ($listenersByPriority as $listener) { - // Performance note: after some testing, it appears that accumulating listeners and sending - // them at the end of the method is FASTER than using generators (ie. yielding) - $listenersForEvent[] = $listener; + // Execute listeners + $responses = new ResponseCollection(); + foreach ($listOfListenersByPriority as $listenersList) { + foreach ($listenersList as $listeners) { + foreach ($listeners as $listener) { + $response = $listener($event); + $responses->push($response); + + // If the event was asked to stop propagating, do so + if ($event->propagationIsStopped()) { + $responses->setStopped(true); + break 3; + } + + // If the result causes our validation callback to return true, + // stop propagation + if ($callback && $callback($response)) { + $responses->setStopped(true); + break 3; + } + } } } - return $listenersForEvent; + return $responses; } } diff --git a/src/SharedEventManager.php b/src/SharedEventManager.php index 2b546b7..f1cd58c 100644 --- a/src/SharedEventManager.php +++ b/src/SharedEventManager.php @@ -190,7 +190,7 @@ public function getListeners(array $identifiers, $eventName) } } - if (isset($this->identifiers['*']) && ! in_array('*', $identifiers, true)) { + if (isset($this->identifiers['*'])) { $wildcardIdentifier = $this->identifiers['*']; if (isset($wildcardIdentifier[$eventName])) { foreach ($wildcardIdentifier[$eventName] as $p => $l) { diff --git a/test/EventManagerTest.php b/test/EventManagerTest.php index eb50cc7..a422211 100644 --- a/test/EventManagerTest.php +++ b/test/EventManagerTest.php @@ -56,7 +56,12 @@ public function getListenersForEvent($event, EventManager $manager) $r->setAccessible(true); $events = $r->getValue($manager); - return isset($events[$event]) ? $events[$event] : []; + $listenersByPriority = isset($events[$event]) ? $events[$event] : []; + foreach ($listenersByPriority as $priority => & $listeners) { + $listeners = $listeners[0]; + } + + return $listenersByPriority; } public function testAttachShouldAddListenerToEvent() From 9d10eecc4141710afe5c17f13962e556ed3af4f0 Mon Sep 17 00:00:00 2001 From: Marc Bennewitz Date: Sat, 9 Jul 2016 09:03:46 +0200 Subject: [PATCH 3/5] Performance increasement for MultipleEventMultipleSharedListener --- src/EventManager.php | 70 ++++++++++---------- src/SharedEventManager.php | 42 +++++++----- src/Test/EventListenerIntrospectionTrait.php | 11 +-- 3 files changed, 66 insertions(+), 57 deletions(-) diff --git a/src/EventManager.php b/src/EventManager.php index 7e24d96..910db78 100644 --- a/src/EventManager.php +++ b/src/EventManager.php @@ -22,6 +22,22 @@ class EventManager implements EventManagerInterface /** * Subscribed events and their listeners * + * STRUCTURE: + * [ + * => [ + * => [ + * 0 => [, ...] + * ], + * ... + * ], + * ... + * ] + * + * NOTE: + * This structure helps us to reuse the list of listeners + * instead of first iterating over it and generating a new one + * -> In result it improves performance by up to 25% even if it looks a bit strange + * * @var array[] */ protected $events = []; @@ -164,10 +180,7 @@ public function attach($eventName, callable $listener, $priority = 1) )); } - // see performance note of triggerListeners() why the internal - // event array is structured this way. $this->events[$eventName][(int) $priority][0][] = $listener; - return $listener; } @@ -264,38 +277,23 @@ protected function triggerListeners(EventInterface $event, callable $callback = throw new Exception\RuntimeException('Event is missing a name; cannot trigger!'); } - // Merge all listeners by priority together with format - // [ - // => [ - // [ - // [, , ...] - // ], - // ... - // ], - // => [ - // [ - // [, , ...] - // ], - // ... - // ], - // ... - // ] - // - // PERFORMANCE NOTE: - // This way pushing the list of listeners into a new generated list - // helps us to not call "array_merge_[recursive]" - // and reusing the list of listeners we already have - // instead of iterating over it and generating a new one. - // -> In result it improves performance by up to 25% even if it looks a bit strange. - $listOfListenersByPriority = isset($this->events[$name]) ? $this->events[$name] : []; - if (isset($this->events['*'])) { - foreach ($this->events['*'] as $p => $l) { - $listOfListenersByPriority[$p][] = $l[0]; + if (isset($this->events[$name])) { + $listOfListenersByPriority = $this->events[$name]; + + if (isset($this->events['*'])) { + foreach ($this->events['*'] as $priority => $listOfListeners) { + $listOfListenersByPriority[$priority][] = $listOfListeners[0]; + } } + } elseif (isset($this->events['*'])) { + $listOfListenersByPriority = $this->events['*']; + } else { + $listOfListenersByPriority = []; } + if ($this->sharedManager) { - foreach ($this->sharedManager->getListeners($this->identifiers, $name) as $p => $l) { - $listOfListenersByPriority[$p][] = $l; + foreach ($this->sharedManager->getListeners($this->identifiers, $name) as $priority => $listeners) { + $listOfListenersByPriority[$priority][] = $listeners; } } @@ -307,8 +305,8 @@ protected function triggerListeners(EventInterface $event, callable $callback = // Execute listeners $responses = new ResponseCollection(); - foreach ($listOfListenersByPriority as $listenersList) { - foreach ($listenersList as $listeners) { + foreach ($listOfListenersByPriority as $listOfListeners) { + foreach ($listOfListeners as $listeners) { foreach ($listeners as $listener) { $response = $listener($event); $responses->push($response); @@ -316,14 +314,14 @@ protected function triggerListeners(EventInterface $event, callable $callback = // If the event was asked to stop propagating, do so if ($event->propagationIsStopped()) { $responses->setStopped(true); - break 3; + return $responses; } // If the result causes our validation callback to return true, // stop propagation if ($callback && $callback($response)) { $responses->setStopped(true); - break 3; + return $responses; } } } diff --git a/src/SharedEventManager.php b/src/SharedEventManager.php index f1cd58c..371b5d9 100644 --- a/src/SharedEventManager.php +++ b/src/SharedEventManager.php @@ -153,8 +153,8 @@ public function detach(callable $listener, $identifier = null, $eventName = null /** * Retrieve all listeners for a given identifier and event * - * @param array $identifiers - * @param string $eventName + * @param string[] $identifiers + * @param string $eventName * @return array[] * @throws Exception\InvalidArgumentException */ @@ -167,7 +167,7 @@ public function getListeners(array $identifiers, $eventName) )); } - $listeners = []; + $returnListeners = []; foreach ($identifiers as $identifier) { if ('*' === $identifier || ! is_string($identifier) || empty($identifier)) { @@ -177,15 +177,17 @@ public function getListeners(array $identifiers, $eventName) )); } - $listenersByIdentifier = isset($this->identifiers[$identifier]) ? $this->identifiers[$identifier] : []; - if (isset($listenersByIdentifier[$eventName])) { - foreach ($listenersByIdentifier[$eventName] as $p => $l) { - $listeners[$p] = isset($listeners[$p]) ? array_merge($listeners[$p], $l) : $l; + if (isset($this->identifiers[$identifier])) { + $listenersByIdentifier = $this->identifiers[$identifier]; + if (isset($listenersByIdentifier[$eventName])) { + foreach ($listenersByIdentifier[$eventName] as $priority => $listeners) { + $returnListeners[$priority][] = $listeners; + } } - } - if (isset($listenersByIdentifier['*'])) { - foreach ($listenersByIdentifier['*'] as $p => $l) { - $listeners[$p] = isset($listeners[$p]) ? array_merge($listeners[$p], $l) : $l; + if (isset($listenersByIdentifier['*'])) { + foreach ($listenersByIdentifier['*'] as $priority => $listeners) { + $returnListeners[$priority][] = $listeners; + } } } } @@ -193,18 +195,24 @@ public function getListeners(array $identifiers, $eventName) if (isset($this->identifiers['*'])) { $wildcardIdentifier = $this->identifiers['*']; if (isset($wildcardIdentifier[$eventName])) { - foreach ($wildcardIdentifier[$eventName] as $p => $l) { - $listeners[$p] = isset($listeners[$p]) ? array_merge($listeners[$p], $l) : $l; + foreach ($wildcardIdentifier[$eventName] as $priority => $listeners) { + $returnListeners[$priority][] = $listeners; } } if (isset($wildcardIdentifier['*'])) { - foreach ($wildcardIdentifier['*'] as $p => $l) { - $listeners[$p] = isset($listeners[$p]) ? array_merge($listeners[$p], $l) : $l; + foreach ($wildcardIdentifier['*'] as $priority => $listeners) { + $returnListeners[$priority][] = $listeners; } } } - return $listeners; + foreach ($returnListeners as $priority => $listOfListeners) { + // Argument unpacking requires PHP-5.6 + // $listeners[$priority] = array_merge(...$listOfListeners); + $returnListeners[$priority] = call_user_func_array('array_merge', $listOfListeners); + } + + return $returnListeners; } /** @@ -212,7 +220,7 @@ public function getListeners(array $identifiers, $eventName) */ public function clearListeners($identifier, $eventName = null) { - if (! array_key_exists($identifier, $this->identifiers)) { + if (! isset($this->identifiers[$identifier])) { return false; } diff --git a/src/Test/EventListenerIntrospectionTrait.php b/src/Test/EventListenerIntrospectionTrait.php index 4f66766..35f3739 100644 --- a/src/Test/EventListenerIntrospectionTrait.php +++ b/src/Test/EventListenerIntrospectionTrait.php @@ -66,13 +66,16 @@ private function getListenersForEvent($event, EventManager $events, $withPriorit { $r = new ReflectionProperty($events, 'events'); $r->setAccessible(true); - $listeners = $r->getValue($events); + $internal = $r->getValue($events); - if (! isset($listeners[$event])) { - return $this->traverseListeners([]); + $listeners = []; + foreach (isset($internal[$event]) ? $internal[$event] : [] as $p => $listOfListeners) { + foreach ($listOfListeners as $l) { + $listeners[$p] = isset($listeners[$p]) ? array_merge($listeners[$p], $l) : $l; + } } - return $this->traverseListeners($listeners[$event], $withPriority); + return $this->traverseListeners($listeners, $withPriority); } /** From 29d9577836c3cfc64c80ca7a0f1822e83b62a2c2 Mon Sep 17 00:00:00 2001 From: Marc Bennewitz Date: Tue, 12 Jul 2016 08:42:05 +0200 Subject: [PATCH 4/5] set target and params only if any --- src/EventManager.php | 35 +++++++++++++++++++++++------------ 1 file changed, 23 insertions(+), 12 deletions(-) diff --git a/src/EventManager.php b/src/EventManager.php index 910db78..9d7c1d1 100644 --- a/src/EventManager.php +++ b/src/EventManager.php @@ -132,8 +132,14 @@ public function trigger($eventName, $target = null, $argv = []) { $event = clone $this->eventPrototype; $event->setName($eventName); - $event->setTarget($target); - $event->setParams($argv); + + if ($target !== null) { + $event->setTarget($target); + } + + if ($argv) { + $event->setParams($argv); + } return $this->triggerListeners($event); } @@ -145,8 +151,14 @@ public function triggerUntil(callable $callback, $eventName, $target = null, $ar { $event = clone $this->eventPrototype; $event->setName($eventName); - $event->setTarget($target); - $event->setParams($argv); + + if ($target !== null) { + $event->setTarget($target); + } + + if ($argv) { + $event->setParams($argv); + } return $this->triggerListeners($event, $callback); } @@ -226,22 +238,21 @@ public function detach(callable $listener, $eventName = null, $force = false) break; } } + } - // If the queue for the given event is empty, remove it. - if (empty($this->events[$eventName])) { - unset($this->events[$eventName]); - break; - } + // If the queue for the given event is empty, remove it. + if (empty($this->events[$eventName])) { + unset($this->events[$eventName]); } } /** * @inheritDoc */ - public function clearListeners($event) + public function clearListeners($eventName) { - if (isset($this->events[$event])) { - unset($this->events[$event]); + if (isset($this->events[$eventName])) { + unset($this->events[$eventName]); } } From 8a0320c4847d63ac97cc6e9d75f75c6ece122d63 Mon Sep 17 00:00:00 2001 From: Marc Bennewitz Date: Tue, 2 Aug 2016 15:44:16 +0200 Subject: [PATCH 5/5] cs fixes --- src/SharedEventManager.php | 1 - test/TestAsset/StaticEventsMock.php | 1 - 2 files changed, 2 deletions(-) diff --git a/src/SharedEventManager.php b/src/SharedEventManager.php index 371b5d9..b61701b 100644 --- a/src/SharedEventManager.php +++ b/src/SharedEventManager.php @@ -141,7 +141,6 @@ public function detach(callable $listener, $identifier = null, $eventName = null unset($this->identifiers[$identifier][$eventName]); break; } - } // Is the identifier queue now empty? Remove it. diff --git a/test/TestAsset/StaticEventsMock.php b/test/TestAsset/StaticEventsMock.php index 431acc6..dcb9cb9 100644 --- a/test/TestAsset/StaticEventsMock.php +++ b/test/TestAsset/StaticEventsMock.php @@ -30,7 +30,6 @@ public function getListeners($id, $event = null) */ public function attach($identifier, $event, callable $listener, $priority = 1) { - } /**