Skip to content

Conversation

@electronicboy
Copy link
Member

By default, spigot shifts chat over to an unbounded thread pool,
on a normal server, this really offers no gains, the creation of a thread
on submitting to the pool on these servers eats more time vs just running it in
the netty pipeline, however, on servers using plugins which do work in here, there
could be some overall benefits to moving this stuff outside of the pipeline.

In general, this patch does two things:

  1. Exposes the core size for the pool, this allows for ensuring that a number of threads
    sit around in the pool, mitigating the need for creating new threads; This IS however
    caveated, the ThreadPoolExecutor will ONLY create core threads as they're needed, it
    just won't allow for us to dip back under the # of core threads, this can potentially
    be mitigated by calling prestartCoreThread, however, I'm not sure if there is much justification
    for this
  2. Exposes a max size for the pool, as stated, by default, this is unbounded, for most
    servers limiting the size of the pool is going to have 0 effects given how fast chat
    is actually processed, this is honestly really just exposed for the misnomers or people
    who just wanna ensure that this won't grow over a specific size if chat gets stupidly active

This is basically my own take on #7483

@NoahvdAa
Copy link
Member

NoahvdAa commented Aug 6, 2022

Spigot no longer uses a thread pool for chat as of the 1.19.1 update.

@NoahvdAa NoahvdAa closed this Aug 6, 2022
@NoahvdAa
Copy link
Member

NoahvdAa commented Aug 6, 2022

Closed this too quickly, apologies. In 1.19.1, a thread pool is still used, but it was moved into CraftBukkit, into the MinecraftServer class.

By default, spigot shifts chat over to an unbounded thread pool,
on a normal server, this really offers no gains, the creation of a thread
on submitting to the pool on these servers eats more time vs just running it in
the netty pipeline, however, on servers using plugins which do work in here, there
could be some overall benefits to moving this stuff outside of the pipeline.

In general, this patch does two things:
1) Exposes the core size for the pool, this allows for ensuring that a number of threads
sit around in the pool, mitigating the need for creating new threads; This IS however
caveated, the ThreadPoolExecutor will ONLY create core threads as they're needed, it
just won't allow for us to dip back under the # of core threads, this can potentially
be mitigated by calling prestartCoreThread, however, I'm not sure if there is much justification
for this
2) Exposes a max size for the pool, as stated, by default this is unbounded, for most
servers limiting the size of the pool is going to have 0 effects given how fast chat
is actually processed, this is honestly really just exposed for the misnomers or people
who just wanna ensure that this won't grow over a specific size if chat gets stupidly active
@electronicboy electronicboy force-pushed the feature/chat-thread-limits branch from 6a8d1b0 to 540fe6f Compare September 18, 2022 22:01
@electronicboy electronicboy requested a review from a team as a code owner September 18, 2022 22:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants