Rosewood-Development / PlayerParticles

The PlayerParticles plugin made by Esophose
Other
172 stars 49 forks source link

PP Reload causes TPS to drop. #69

Closed mibby closed 3 years ago

mibby commented 3 years ago

PlayerParticles dev 51 https://ci.codemc.io/job/Esophose/job/PlayerParticles/

Reloading PlayerParticles with /pp reload causes TPS to take a massive hit. Main thread freezing (tps dropping to ~10-15) until it's done reloading / initializing (every single time you try). Caused by establishing database connection?

[04:37:06] [Server thread/INFO]: mibby issued server command: /pp reload
[04:37:06] [Server thread/INFO]: [PlayerParticles] Data handler connected using MySQL.
[04:37:09] [Server thread/INFO]: [PlayerParticles] Reloaded configuration.

Config https://paste.gg/p/anonymous/40e0aa9b7143441ca9f4777a4ca1bb46/files/ba58cf3b174b4b3ca2c18136833fde94/raw

Esophose commented 3 years ago

This should be resolved in dev build 52. I was reluctant to do it because of the potential desyncs that can happen, but the only one that happened (that I noticed) was easily resolvable. If you could test this to make sure it works with your database connection that would be great. https://ci.codemc.io/job/Esophose/job/PlayerParticles/54/

mibby commented 3 years ago

It's a little better but there's still a bit of a performance hit when reloading (drop to ~16-17 tps) and takes about 2 seconds to finish initializing. It must be getting hung up on something.

[02:25:30] [Server thread/INFO]: mibby issued server command: /pp reload
[02:25:30] [Craft Scheduler Thread - 153 - PlayerParticles/INFO]: [PlayerParticles] Data handler connected using MySQL.
[02:25:32] [Craft Scheduler Thread - 130 - PlayerParticles/INFO]: [PlayerParticles] Reloaded configuration.
[02:25:45] [Server thread/INFO]: mibby issued server command: /pp reload
[02:25:45] [Craft Scheduler Thread - 132 - PlayerParticles/INFO]: [PlayerParticles] Data handler connected using MySQL.
[02:25:47] [Craft Scheduler Thread - 146 - PlayerParticles/INFO]: [PlayerParticles] Reloaded configuration.
Esophose commented 3 years ago

The message simply prints out 10 ticks after the reload command is run. https://prnt.sc/ytve0t Any sort of performance hit you're seeing should be written off as perfectly acceptable, it is reloading the entire plugin after all. Having the server slow down for less than a single tick isn't really a problem.

mibby commented 3 years ago

Is there any way for me to expose that debug timer when reloading or is that a custom debug build? It really feels like it takes longer than 21ms and dropping performance down to 16 tps until it completes is not ideal. Reloading more gracefully would be preferred.

Other SQL database plugins I use and reload don't impact performance.

Esophose commented 3 years ago

I just temporarily added the message there. Here's a build with it playerparticles-7.19-reload-stats.zip

mibby commented 3 years ago

How weird. The reload claims to only take 15-17ms. The TPS drop starts to happen after that output. Running a timings check, it seems to be caused by PlayerParticle's PermissionManager.

link

Esophose commented 3 years ago

Can you run a spark profiler report just for that reload command? I'm not exactly sure what's causing that to happen as permission checks are usually supposed to be really fast. The plugin is called spark on Spigot and you can run the profiler with /spark profiler --timeout 30 and run the reload command right after that, it'll tell me exactly what's causing the slowdown

mibby commented 3 years ago

Here's a single /pp reload at the beginning of a 30 second spark profile. https://spark.lucko.me/#SVb0llT9WL

Here is a 30 second spark profile with me reloading pp constantly every time after it has outputted the plugin has been reloaded message. https://spark.lucko.me/#cujtH6sI9K

Esophose commented 3 years ago

Looks like it was taking a long time to register all the permissions, here's a build with that removed so it's only done on startup now https://ci.codemc.io/job/Esophose/job/PlayerParticles/55/

mibby commented 3 years ago

Fix seems to be working great, thanks! No more major tps hit when reloading PP.