Skip to content

Use a proper thread pool in ServerLoginPacketListenerImpl and MCUtil #8798

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

egg82
Copy link
Contributor

@egg82 egg82 commented Jan 18, 2023

Fixes #8797

An arbitrary-ish number of 1024 was chosen for the bounded queue size.
The asyncExecutor in MCUtil was left unbounded.

@egg82 egg82 requested a review from a team as a code owner January 18, 2023 19:11
Copy link
Member

@NoahvdAa NoahvdAa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try to avoid reformatting these large blocks to keep the diff small :)

This should probably also go into the original patch.

@electronicboy
Copy link
Member

electronicboy commented Jan 18, 2023

  1. You mangled the formatting
  2. core size of 16 is excessive, especially as the thread pool will race towards that needlessly; I'd probably just go for like 4 or something, and maybe make it configurable; ofc, this is actually probably a pretty good target for green threads in the future

@lynxplay
Copy link
Contributor

Are you still planning on moving forward with this ? I don't recall the dhrama discussion about it but I'd close this otherwise.

@lynxplay
Copy link
Contributor

lynxplay commented Aug 6, 2023

Closing this given it has gone rather stale 😓 (love you egg <3)

@lynxplay lynxplay closed this Aug 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Closed
Development

Successfully merging this pull request may close these issues.

ServerLoginPacketListenerImpl uses Executors.newCachedThreadPool with unbounded queue
4 participants