# Reassessing PlayerMoveEvent: Exploding the myths of its performance impacts

Discussion in 'Resources' started by SOFe, Apr 15, 2018.

Messages:
1,968
GitHub:
sof3
A lot of people complain that PlayerMoveEvent should not be used because it's called frequently and lags your server.

One important fact to note is that whether you handle PlayerMoveEvent or not, PocketMine will call it regardless. Let's first study the stack trace of handling a PlayerMoveEvent:
Code:
#0 C:\sofe\sand\pm3\src\pocketmine\plugin\MethodEventExecutor.php(38): testMove->m(Object(pocketmine\event\player\PlayerMoveEvent))
#1 C:\sofe\sand\pm3\src\pocketmine\plugin\RegisteredListener.php(98): pocketmine\plugin\MethodEventExecutor->execute(Object(testMove), Object(pocketmine\event\player\PlayerMoveEvent))
#2 C:\sofe\sand\pm3\src\pocketmine\plugin\PluginManager.php(699): pocketmine\plugin\RegisteredListener->callEvent(Object(pocketmine\event\player\PlayerMoveEvent))
#3 C:\sofe\sand\pm3\src\pocketmine\Player.php(1595): pocketmine\plugin\PluginManager->callEvent(Object(pocketmine\event\player\PlayerMoveEvent))
#4 C:\sofe\sand\pm3\src\pocketmine\Player.php(1702): pocketmine\Player->processMovement(1)
#5 C:\sofe\sand\pm3\src\pocketmine\level\Level.php(762): pocketmine\Player->onUpdate(212)
#6 C:\sofe\sand\pm3\src\pocketmine\Server.php(2349): pocketmine\level\Level->doTick(212)

#3 (PluginManager->callEvent(PlayerMoveEvent) will be called anyway.
For each listener, #2, #1 and #0 will be called respectively.

Now let's calculate how much it takes to call an empty PlayerMoveEvent handler, starting from #3.
callEvent() first checks for eventCallDepth, which is O(1) under normal circumstances.

Then it retrieves the linked HandlerList list of PlayerMoveEvent. It calls HandlerList::getHandlerListFor(string), which consists of the following steps:
• If the HandlerList was initialized before, it fetches the handler list from the HandlerList::$handlerList map. In PHP, this should be implemented as a hash map, so I assume this is in almost O(1) time. • If it wasn't, the HandlerList will be initialized, and it will be stored in HandlerList::$handlerList. Therefore, only the first PlayerMoveEvent call will be slightly slower, which is not the focus of our discussion.
A simple summary of HandlerList::getHandlerListFor(string): it doesn't depend on the number of handlers registered for the event.

Next, there is an assertion and ++$this->eventCallDepth, both of which in O(1). (More precisely, assertion is O(0) on production servers) And then we start looping EventPriority::ALL. There are currently 6 priorities, and this will not increase/decrease because of the number of handlers. Then there is a while loop (actually a for loop, not sure why @dktapps wrote it so uglily) traversing the linked list of HandlerLists through HandlerList->getParent(). This depends on the defined class inheritance and whether the classes are abstract/@allowHandle/non-abstract, and independent from whether plugins register listeners for PlayerMoveEvent. (For PlayerMoveEvent, getParent() returns null the first time so the loop only runs once) Now comes the part that matters: foreach(HandlerList->getListenersByPriority(int)). The number of iterations depends on the number of handlers registered for that event. And it consists of the following calls: • Check if plugin is enabled (negligible boolean getter) • Call RegisteredListener->callEvent() • Check cancelled state (negligible boolean getters) • Init timings (empty function if timings not enabled) • Stop timings (also conditionally empty) • MethodEventExecutor->execute() • This calls Listener->{$method}($event). This might cause small problems in terms of reflections, but I am not sure about how PHP calls class methods internally. Regardless, the performance impact should be finite. • Eventually calls your listener. In short, the performance impact of calling an event handler itself is very small! So why are we so concerned about using PlayerMoveEvent? The main reason is that plugins that only want to know whether a player is inside an area are creating too heavy burden on this event. For example, a faction plugin that only wants to show the "you entered wilderness" message check for faction chunks every tick, which is causing totally unnecessary performance cost. The correct solution would be to store the previous faction that owns the player's current chunk (not the exact position!) and only check it occasionally. For some other purposes, such as disallowing certain movement, using PlayerMoveEvent is the perfect solution. If you try to store the previous solution to restore movement, you're merely reimplementing the logic at an unnecessary cost. The problem is not in the event handler itself. It's in whether you're doing too much. It's actually also a valid solution to check if$server->getTick()%10 to rate limit the execution, although this might result in missing some movements.

Here is a search of plugins on Poggit using PlayerMoveEvent, for reference:
https://poggit.pmmp.io/grepPlugins/2018-04-15-db9fbf4a1f939029.html

2. ### yuko fuyutsukiSlime

Messages:
77
GitHub:
fuyutsuki
It is correct.
PlayerMoveEvent itself is not slow, but it is showing up late by a shitty process that is tied to it.

Messages:
1,968
GitHub:
sof3
You should still not underestimate its performance impacts when you execute CPU-intensive code. For a 50-slot server, PlayerMoveEvent can be called up to 1000 times per second.

Messages:
77
GitHub:
fuyutsuki
I think so.

5. ### Tee7evenSlime

Messages:
81
GitHub:
tee7even
What do you think would be an appropriate way to check if player has entered a nether portal?

Messages:
1,968
GitHub:
sof3
For creative mode: you want to teleport the player after he enters the portal immediately. You can eliminate most event calls by checking if the interacting block is a portal block really quickly. Therefore, PlayerMoveEvent is appropriate.
For survival mode: there is a delay and tick counter anyway, so there are no drawbacks in doing it with the task-check method.

For supporting both modes: if you were to generalize both of them, task check is easier to implement both together.

7. ### DanikthebossBaby Zombie

Messages:
144
GitHub:
daniktheboss
Let's say, we have an array with positions. And we check if the player is in the area every move event. Is that efficient or is there a better way to do such?

8. ### SandertvZombie PigmanPoggit Reviewer

Messages:
786
GitHub:
Sandertv
A single array access is really not heavy. Don't waste your time doing micro optimizations on that.

9. ### DanikthebossBaby Zombie

Messages:
144
GitHub:
daniktheboss
So what's a scenario that would abuse that?

Messages:
1,968
GitHub:
sof3
It could be heavy if you have thousands of areas, or if your area comparison algorithm is heavy (e.g. a lot of trig, log and sqrt).
Or a factions plugin that obtains chunk information from a database. Especially a remote one, taking something tiny like 5 milliseconds (this would already be extremely fast for a remote MySQL query). 20 players moving in this tick, so you take 100 milliseconds to execute the queries. This will result in 200% TPS load.

11. ### DanikthebossBaby Zombie

Messages:
144
GitHub:
daniktheboss
So to overcome that, you can cache the faction claims, and then check from that cache array?

Messages:
1,968
GitHub:
sof3
Yes, that's the best way. But if the data aren't already in the cache (or you clear the cache after some time), you could store the from-position, execute the async query with the to-position, and if the to-position is entry-forbidden, teleport back the player to the from-position.

13. ### DanikthebossBaby Zombie

Messages:
144
GitHub:
daniktheboss
Alright, thanks for the info