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

Use RetainableByteBuffer in HttpConnection #6322

Closed
gregw opened this issue May 25, 2021 · 3 comments · Fixed by #6332
Closed

Use RetainableByteBuffer in HttpConnection #6322

gregw opened this issue May 25, 2021 · 3 comments · Fixed by #6332

Comments

@gregw
Copy link
Contributor

gregw commented May 25, 2021

The org.eclipse.jetty.server.HttpConnection class should use org.eclipse.jetty.io.RetainableByteBuffer rather than it's own reference counting of buffers. This will eventually allow a more efficient buffer pool abstraction to be used that provides RetainableByteBuffers directly.

@gregw gregw added this to To do in Jetty 10.0.4/11.0.4 FROZEN via automation May 25, 2021
@gregw
Copy link
Contributor Author

gregw commented May 25, 2021

@lorban Simone suggested that we do this. I had a quick look at the input side at least looks achievable, so I thought I'd punt to you with your HttpInput experience. I think it can be done entirely in HttpConnection since every increment of the reference counter is done by calling newContent, so the Content itself can do the retain and release. I've not yet looked at the output side.

@sbordet
Copy link
Contributor

sbordet commented May 25, 2021

@gregw I don't think the output side is involved at all, as RetainableByteBuffer is only for the input side.
By using it, we will make all protocol implementations homogeneous and we could pool them more efficiently.

On the output side, all implementations currently rely on pooled "naked" ByteBuffers but I'm not sure how much better we can do considering that we have to wrap byte[] most of the times.

@sbordet sbordet removed this from To do in Jetty 10.0.4/11.0.4 FROZEN Jun 4, 2021
@lorban lorban linked a pull request Jul 8, 2021 that will close this issue
gregw added a commit that referenced this issue Jul 12, 2021
Add a Log2 bucket size imple for ArrayRetainableByteBufferPool

Signed-off-by: Greg Wilkins <gregw@webtide.com>
gregw added a commit that referenced this issue Jul 12, 2021
Add a Log2 bucket size impl for ArrayRetainableByteBufferPool

Signed-off-by: Greg Wilkins <gregw@webtide.com>
@gregw gregw linked a pull request Jul 12, 2021 that will close this issue
lorban added a commit that referenced this issue Jul 12, 2021
Signed-off-by: Ludovic Orban <lorban@bitronix.be>
gregw added a commit that referenced this issue Jul 13, 2021
Dumpable

Signed-off-by: Greg Wilkins <gregw@webtide.com>
gregw added a commit that referenced this issue Jul 13, 2021
Sampled Pool

Signed-off-by: Greg Wilkins <gregw@webtide.com>
lorban added a commit that referenced this issue Jul 24, 2021
Signed-off-by: Ludovic Orban <lorban@bitronix.be>
gregw added a commit that referenced this issue Jul 25, 2021
Add a exponential bucket size impl to test ArrayRetainableByteBufferPool extensibility

Signed-off-by: Greg Wilkins <gregw@webtide.com>
gregw added a commit that referenced this issue Jul 26, 2021
Add a exponential bucket size impl to test ArrayRetainableByteBufferPool extensibility

Signed-off-by: Greg Wilkins <gregw@webtide.com>
@gregw
Copy link
Contributor Author

gregw commented Jul 26, 2021

Once the next release is out, we need to ask for a few ByteBufferPool dumps from real deployments so we can plot histograms of buffer sizes in the pool.

@lorban lorban added this to To do in Jetty 10.0.7/11.0.7 FROZEN via automation Jul 26, 2021
@lorban lorban moved this from To do to Done in Jetty 10.0.7/11.0.7 FROZEN Jul 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
3 participants