GH-437 Improve schedulers

[BlackBaroness]

Sorry for being late! Will finish soon! Closes #437.

I am currently reviewing and improving schedulers for all platforms:

• Core
• Bukkit
• BungeeCord
• Sponge
• Velocity
• Fabric
• Jda
• Minestom

About BungeeCord

BungeeCord's own scheduler is not efficient because it creates an unlimited cached thread pool for
each plugin and just uses it:
https://github.com/SpigotMC/BungeeCord/blob/master/proxy/src/main/java/net/md_5/bungee/scheduler/BungeeScheduler.java
https://github.com/SpigotMC/BungeeCord/blob/master/api/src/main/java/net/md_5/bungee/api/plugin/Plugin.java

It also just blocks the threads if a task with a delay is received (by using Thread.sleep),
so the thread won't be doing anything useful even if there is more tasks available

I decided to use the Core scheduler instead, since it's more efficient by at least providing a max cap

About Velocity

Velocity's own scheduler is very similar to BungeeCord's one
Each plugin has it's own uncapped cached thread pool
That doesn't look good
https://github.com/PaperMC/Velocity/blob/dev/3.0.0/proxy/src/main/java/com/velocitypowered/proxy/plugin/loader/VelocityPluginContainer.java
https://github.com/PaperMC/Velocity/blob/dev/3.0.0/api/src/main/java/com/velocitypowered/api/plugin/PluginContainer.java

I decided to use the Core scheduler instead, since it's more efficient by at least providing a max cap

About Bukkit

Looks good, only small improvements were made

About Minestom

Don't see anything to improve, it's fine. Also the Minestom's own scheduler is good, so no need to use our own

About Sponge

Sponge provides a scheduler similar to Bukkit, so I implemented a wrapper for one

About JDA

I don't think there is a specific implementation for JDA, so we should just use the one from Core

About Fabric

Was implemented by @huanmeng-qwq

[BlackBaroness]

@Rollczi hey, looks like I finished for now, can you help me with Fabric and review the code?

[huanmeng-qwq]

@BlackBaroness Hi, I noticed that you can't solve the fabric platform. Since I don't have permission to submit changes directly to the pr, I will create a new PR and submit it to your fork repository.

[BlackBaroness]

@BlackBaroness Hi, I noticed that you can't solve the fabric platform. Since I don't have permission to submit changes directly to the pr, I will create a new PR and submit it to your fork repository.

Thanks, I merged it

[Rollczi]

Using platform schedulers (BungeeCord, Velocity) can improve performance by sharing unused threads in the cached pools

[BlackBaroness]

Using platform schedulers (BungeeCord, Velocity) can improve performance by sharing unused threads in the cached pools

I believe an uncapped cached pools dedicated to a single plugin only is a bad idea. Especially if you look on how delayed tasks were implemented - they create a thread and then call Thread.sleep inside it, so the pool can grow so much without even doing anything. If this pool was used by the entire proxy, of course it would be nice (like craftbukkit does), but in this case... I feel like the BungeeCord Scheduler is a mistake by design

Velocity is slightly better, but still has an uncapped pool. What if several players will spam async commands?

Proxies are designed to handle thousands of players online concurrently, an uncapped pool can become a disaster when commands are expensive enough to make threads busy

Or, for BungeeCord... Delayed tasks are usually used by cooldown system. What if we have a /login command (what is something all players use at least once) with 2s cooldown? If 200-300 players joined after restart, everyone executed /login (what is async itself) - 200-300 threads were created and then 200-300 threads more created to make delayed tasks (as BungeeCord doesn't use scheduled pools). That's insane

[Rollczi]

Using platform schedulers (BungeeCord, Velocity) can improve performance by sharing unused threads in the cached pools

I believe an uncapped cached pools dedicated to a single plugin only is a bad idea. Especially if you look on how delayed tasks were implemented - they create a thread and then call Thread.sleep inside it, so the pool can grow so much without even doing anything. If this pool was used by the entire proxy, of course it would be nice (like craftbukkit does), but in this case... I feel like the BungeeCord Scheduler is a mistake by design

Velocity is slightly better, but still has an uncapped pool. What if several players will spam async commands?

Proxies are designed to handle thousands of players online concurrently, an uncapped pool can become a disaster when commands are expensive enough to make threads busy

Or, for BungeeCord... Delayed tasks are usually used by cooldown system. What if we have a /login command (what is something all players use at least once) with 2s cooldown? If 200-300 players joined after restart, everyone executed /login (what is async itself) - 200-300 threads were created and then 200-300 threads more created to make delayed tasks (as BungeeCord doesn't use scheduled pools). That's insane

Okay, this is actually true, let's use pool with limited threads.

[Rollczi]

I will try fix some issues

[Rollczi]

@BlackBaroness Thank you for your substantive comments, I implemented them into the old scheduler implementation. I also simplified the code for Farbric scheduler, what do you think, that should work? @huanmeng-qwq

[BlackBaroness]

@Rollczi, looks nice! (not about Fabric)

I noticed you created the SchedulerMainThreadBased class and I believe its naming is pretty strange, how about the MainThreadBasedScheduler?

Also, this new abstraction could be used for Sponge scheduler, I think.

[BlackBaroness]

Also, on Sponge we have both client.scheduler and server.scheduler. I am not sure which one to use, so used the server one. LiteCommands can be installed on clients without a server I guess, it doesn't really depend on server. But how do we specify which scheduler should we use?

We could make it like if (server.available) server else client on platform init, but what if both server and client are available? I am not sure if it's possible, maybe on singleplayer the server runs in completely different process

[Rollczi]

I noticed you created the SchedulerMainThreadBased class and I believe its naming is pretty strange, how about the MainThreadBasedScheduler?

okay, I will rename it

Also, this new abstraction could be used for Sponge scheduler, I think.

I don't think so, this will more complicated... let's follow the principe KISS (Keep it simple stupid)

[Rollczi]

Also, on Sponge we have both client.scheduler and server.scheduler. I am not sure which one to use, so used the server one. LiteCommands can be installed on clients without a server I guess, it doesn't really depend on server. But how do we specify which scheduler should we use?

We could make it like if (server.available) server else client on platform init, but what if both server and client are available? I am not sure if it's possible, maybe on singleplayer the server runs in completely different process

user should use LiteFabricFactory.server() or LiteFabricFactory.client() depends on his implementation

[BlackBaroness]

user should use LiteFabricFactory.server() or LiteFabricFactory.client() depends on his implementation

I talked about Sponge, not Fabric btw
We don't have an implementation-specific builder method there
Now I implemented a AbstractMainThreadBasedScheduler scheduler in Sponge + added an automatic server/client scheduler selection based on what is available. Looks much better now

I believe this PR is ready, if Fabric is ok

[Rollczi]

Thanks for next improvements @BlackBaroness

[Rollczi]

Okay thanks @BlackBaroness and @huanmeng-qwq for your help! ❤️