Skip to content
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

Issue #7078 - share inflater/deflater pools for websocket in jetty 9.4 #7079

Merged

Conversation

lachlan-roberts
Copy link
Contributor

Closes #7078

These changes bring the sharing of the CompressionPools closer to what is done by WebSocketServerComponents in Jetty 10.

  • Share the InflaterPool and DeflaterPool as beans on the Server.
  • Make the default pool size based off the max number of threads from the ThreadPool.
  • Allow pools to be set per context with the ServletContext attributes jetty.websocket.inflater and jetty.websocket.deflater.

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
@joakime
Copy link
Contributor

joakime commented Nov 4, 2021

Make the default pool size based off the max number of threads from the ThreadPool.

Would a user using jakarta.websocket API with streaming message delivery complicate this? (as those cause new threads)

@lachlan-roberts
Copy link
Contributor Author

Would a user using jakarta.websocket API with streaming message delivery complicate this? (as those cause new threads)

@joakime I don't think this is an issue because there will still not be any more than maxThreads going through the permessage-deflate extension at any one time.

The DispatchedMessageSink also uses the ThreadPool so there should still be no thread acquiring two delfaters/inflaters simultaneously. And if this were to happen and the pool was to exceed capacity, it would create non-pooled entries which are discarded on release and it calls the deflater.end() method.

@lachlan-roberts lachlan-roberts added this to In progress in Jetty 9.4.45 - FROZEN via automation Nov 10, 2021
@lachlan-roberts
Copy link
Contributor Author

@gregw @joakime can I get a review on this.

Jetty 9.4.45 - FROZEN automation moved this from In progress to Reviewer approved Nov 16, 2021
@lachlan-roberts lachlan-roberts merged commit 4a497cd into jetty-9.4.x Nov 16, 2021
Jetty 9.4.45 - FROZEN automation moved this from Reviewer approved to Done Nov 16, 2021
@lachlan-roberts lachlan-roberts deleted the jetty-9.4.x-7078-WebSocketCompressionPools branch November 16, 2021 22:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants