Skip to content

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