From d04da9b1d8292e8f5b8ca699ba68917d3b3a8f7c Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Sat, 6 May 2023 15:42:52 +0100 Subject: [PATCH 1/5] Reuse timings handlers for event handlers of the same events due to direct repeated usage of registerEvent() with closures, we've seen some libraries like muqsit/SimplePacketHandler generate very large timings reports, because a new timings handler gets created every time a plugin registers or unregisters a new packet handler callback. This change fixes the problem by ensuring that any handlers derived from the same function, handling the same event class, will share the same timer. --- src/plugin/PluginManager.php | 4 ++-- src/timings/Timings.php | 14 ++++++++++++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/src/plugin/PluginManager.php b/src/plugin/PluginManager.php index d109b23fa..725c915a1 100644 --- a/src/plugin/PluginManager.php +++ b/src/plugin/PluginManager.php @@ -37,7 +37,7 @@ use pocketmine\permission\DefaultPermissions; use pocketmine\permission\PermissionManager; use pocketmine\permission\PermissionParser; use pocketmine\Server; -use pocketmine\timings\TimingsHandler; +use pocketmine\timings\Timings; use pocketmine\utils\AssumptionFailedError; use pocketmine\utils\Utils; use Symfony\Component\Filesystem\Path; @@ -651,7 +651,7 @@ class PluginManager{ throw new PluginException("Plugin attempted to register event handler " . $handlerName . "() to event " . $event . " while not enabled"); } - $timings = new TimingsHandler($handlerName . "(" . (new \ReflectionClass($event))->getShortName() . ")", group: $plugin->getDescription()->getFullName()); + $timings = Timings::getEventHandlerTimings($event, $handlerName, $plugin->getDescription()->getFullName()); $registeredListener = new RegisteredListener($handler, $priority, $plugin, $handleCancelled, $timings); HandlerListManager::global()->getListFor($event)->register($registeredListener); diff --git a/src/timings/Timings.php b/src/timings/Timings.php index a400a73b4..74fa40dde 100644 --- a/src/timings/Timings.php +++ b/src/timings/Timings.php @@ -166,6 +166,8 @@ abstract class Timings{ /** @var TimingsHandler[] */ private static array $events = []; + /** @var TimingsHandler[][] */ + private static array $eventHandlers = []; public static function init() : void{ if(self::$initialized){ @@ -336,4 +338,16 @@ abstract class Timings{ return self::$events[$eventClass]; } + + /** + * @phpstan-template TEvent of Event + * @phpstan-param class-string $event + */ + public static function getEventHandlerTimings(string $event, string $handlerName, string $group) : TimingsHandler{ + if(!isset(self::$eventHandlers[$event][$handlerName])){ + self::$eventHandlers[$event][$handlerName] = new TimingsHandler($handlerName . "(" . self::shortenCoreClassName($event, "pocketmine\\event\\") . ")", group: $group); + } + + return self::$eventHandlers[$event][$handlerName]; + } } From 4caa2c7690464d1be77f53294f6644087f8e3e39 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Sat, 6 May 2023 15:54:11 +0100 Subject: [PATCH 2/5] NetworkSession: send FLYING flag on spectator ability layer fixes #5722 I'm not very clear why this works. PM doesn't use real spectator mode yet (we're still using the faux spectator mode PM has had for years, because I haven't yet assessed how real spectator mode will affect stuff like block interactions), so this ability layer shouldn't have any effect. thank you @Alemiz112 --- src/network/mcpe/NetworkSession.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/network/mcpe/NetworkSession.php b/src/network/mcpe/NetworkSession.php index bff5875b7..1b90547e7 100644 --- a/src/network/mcpe/NetworkSession.php +++ b/src/network/mcpe/NetworkSession.php @@ -894,6 +894,12 @@ class NetworkSession{ [ //TODO: dynamic flying speed! FINALLY!!!!!!!!!!!!!!!!! new AbilitiesLayer(AbilitiesLayer::LAYER_BASE, $boolAbilities, 0.05, 0.1), + + //TODO: HACK! In 1.19.80, the client starts falling in our faux spectator mode when it clips into a + //block. I have no idea why this works, since we don't actually use the real spectator mode. + new AbilitiesLayer(AbilitiesLayer::LAYER_SPECTATOR, [ + AbilitiesLayer::ABILITY_FLYING => true, + ], null, null) ] ))); } From fa715a074ab8af61bae4c0b754785d5f91288563 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Sat, 6 May 2023 16:56:39 +0100 Subject: [PATCH 3/5] Fixed TimingsHandler depth not getting reset when timings is disabled When timings was disabled, internalStopTiming is not called, and timer depth is not decremented. If timings is later reenabled, the next call to internalStartTiming will think the timer is already running, and won't generate any new records for the timer. This has led to broken timings reports with missing Full Server Tick entries, amongst other things. --- src/timings/TimingsHandler.php | 5 +++-- src/timings/TimingsRecord.php | 7 +++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/timings/TimingsHandler.php b/src/timings/TimingsHandler.php index be4979f15..9a8234310 100644 --- a/src/timings/TimingsHandler.php +++ b/src/timings/TimingsHandler.php @@ -111,7 +111,7 @@ class TimingsHandler{ } public static function reload() : void{ - TimingsRecord::clearRecords(); + TimingsRecord::reset(); if(self::$enabled){ self::$timingStart = hrtime(true); } @@ -219,8 +219,9 @@ class TimingsHandler{ /** * @internal */ - public function destroyCycles() : void{ + public function reset() : void{ $this->rootRecord = null; $this->recordsByParent = []; + $this->timingDepth = 0; } } diff --git a/src/timings/TimingsRecord.php b/src/timings/TimingsRecord.php index 5254d2e7d..f09984b5b 100644 --- a/src/timings/TimingsRecord.php +++ b/src/timings/TimingsRecord.php @@ -42,9 +42,12 @@ final class TimingsRecord{ private static ?self $currentRecord = null; - public static function clearRecords() : void{ + /** + * @internal + */ + public static function reset() : void{ foreach(self::$records as $record){ - $record->handler->destroyCycles(); + $record->handler->reset(); } self::$records = []; self::$currentRecord = null; From 325ffec1be433cd0ad6321044d23f0efd3e6efee Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Sat, 6 May 2023 17:01:49 +0100 Subject: [PATCH 4/5] Release 4.20.3 --- changelogs/4.20.md | 13 +++++++++++++ src/VersionInfo.php | 2 +- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/changelogs/4.20.md b/changelogs/4.20.md index 4a91ab440..bc8c931b2 100644 --- a/changelogs/4.20.md +++ b/changelogs/4.20.md @@ -51,3 +51,16 @@ Released 4th May 2023. ## Fixes - Fixed all types of wooden logs appearing as oak in the inventory. - Fixed a performance issue in `BaseInventory->canAddItem()` (missing `continue` causing useless logic to run). + +# 4.20.3 +Released 6th May 2023. + +## Improvements +- Reduced memory usage of `RuntimeBlockMapping` from 25 MB to 9 MB. Since every thread has its own copy of the block map, this saves a substantial amount of memory. + +## Fixes +- Fixed players falling through blocks in spectator mode. +- Fixed timings reports getting bloated by prolific usage of `PluginManager->registerEvent()`. + - This was caused by creating a new timings handler for each call, regardless of whether a timer already existed for the given event and callback. +- Fixed `Full Server Tick` and other records being missing from timings reports. + - This was caused by timings handler depth not getting reset when timings was disabled and later re-enabled. diff --git a/src/VersionInfo.php b/src/VersionInfo.php index df44e6982..879ea817c 100644 --- a/src/VersionInfo.php +++ b/src/VersionInfo.php @@ -32,7 +32,7 @@ use function str_repeat; final class VersionInfo{ public const NAME = "PocketMine-MP"; public const BASE_VERSION = "4.20.3"; - public const IS_DEVELOPMENT_BUILD = true; + public const IS_DEVELOPMENT_BUILD = false; public const BUILD_CHANNEL = "stable"; private function __construct(){ From 3b893961e4414f66686dfbb2a6860043e0235552 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Sat, 6 May 2023 17:01:49 +0100 Subject: [PATCH 5/5] 4.20.4 is next --- src/VersionInfo.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/VersionInfo.php b/src/VersionInfo.php index 879ea817c..0b72d7f9e 100644 --- a/src/VersionInfo.php +++ b/src/VersionInfo.php @@ -31,8 +31,8 @@ use function str_repeat; final class VersionInfo{ public const NAME = "PocketMine-MP"; - public const BASE_VERSION = "4.20.3"; - public const IS_DEVELOPMENT_BUILD = false; + public const BASE_VERSION = "4.20.4"; + public const IS_DEVELOPMENT_BUILD = true; public const BUILD_CHANNEL = "stable"; private function __construct(){