PaperMC / Waterfall

BungeeCord fork that aims to improve performance and stability.
https://papermc.io
MIT License
743 stars 296 forks source link

Reimplement BungeeScheduler #541

Open Techcable opened 4 years ago

Techcable commented 4 years ago

Bungee Scheduler is a little odd. Each plugin gets its own instance of the JDK Executors.cachedThreadExecutor().

If you want delayed or periodic tasks, each of the tasks calls Thread.sleep and blocks until the appropriate time.

Blocking these tasks seems like poor practice. Each scheduled task takes up a dedicated thread, even though it may only run very rarely or for a very short period.

We should consider either

  1. Making BungeeScheduler a simple wrapper around JDK's SchedulerdExecutorService
    • @electronicboy was thinking about this possibility while he was on the toilet
    • ScheduledExecutorService has a fixed number of threads that can never grow. Is this a problem?
    • Should each plugin get a dedicated scheduler or should they share a global one?
    • If we go with the fixed-number of threads and a global scheduler, then a badly behaved (blocking)
    • Theoretically, if the plugin spawns many tasks that do blocking IO this could be a breaking change. The scheduler would only spawn a fixed number of threads instead of growing to fit all the blocking IO tasks
  2. Reimplement BungeeScheduler using queueing backed by a
    • A delayed (or periodic) task would just push itself to the back of the queue if it wanted to be re-run instead of blocking the thread
    • Unlike a global queue, this would allow more threads to be created in the presence of many blocking tasks
    • Unlike a dedicated per-thread queue, this would avoid waste if a plugin doesn't use the scheduler
    • This would avoid breaking the API in the hypothetical case a plugin spanws many blocking tasks
Techcable commented 4 years ago

I'm leaning towards making the scheduler just a simple wrapper around the JDK's ScheduledExecutorService. It should be per-plugin so plugins don't interfere with each others tasks. I assume wasted threads for plugins that don't use the scheduler isn't a problem.

There is technically potential for API if a plugin uses a lot of blocking tasks but those plugins are kind of dumb and I don't know that we should support them.

If we are worried about potential breakage, we should probably just reimplement the whole thing from scratch. Seems like netty does something like this

astei commented 4 years ago

An idea: Velocity implements its scheduler by using a dedicated executor alongside a single-threaded scheduled executor to fire periodic tasks.