-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Fixes #12227 - Improve HttpConnection buffer recycling. #12237
Fixes #12227 - Improve HttpConnection buffer recycling. #12237
Conversation
Alternative to #12228. In this PR, the responsibility to release the buffers is in 2 methods: onFillable() (called when network data is available, and to process the next request) and parseAndFillForContent() (called from Request.read()). Signed-off-by: Simone Bordet <[email protected]>
Signed-off-by: Simone Bordet <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with the approach taken.... just now play whack-a-bug until it works
jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpConnection.java
Show resolved
Hide resolved
jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpConnection.java
Show resolved
Hide resolved
jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpConnection.java
Show resolved
Hide resolved
Using the request executor to dispatch onFillable(). Signed-off-by: Simone Bordet <[email protected]>
Ensure there is a request before accessing components
Release buffer after consumeAvailable
@sbordet I've been playing whack-a-bug with this, but getting closer.... |
Still problems
@sbordet If I try to add a release in |
…ary. Fixed failing tests that were not completing the Handler Callback. Signed-off-by: Simone Bordet <[email protected]>
@gregw I think the current handling is correct. It is the test that does not complete the I pushed a fix to the tests to complete the callback, and the tests locally pass for me. |
...nsports/src/test/java/org/eclipse/jetty/test/client/transport/HttpClientIdleTimeoutTest.java
Show resolved
Hide resolved
Just improving the test code. The flakyness was likely fixed by the work in #12216 and #12237. Signed-off-by: Simone Bordet <[email protected]>
Just improving the test code. The flakyness was likely fixed by the work in #12216 and #12237. Signed-off-by: Simone Bordet <[email protected]>
Alternative to #12228.
In this PR, the responsibility to release the buffers is in 2 methods: onFillable() (called when network data is available, and to process the next request) and parseAndFillForContent() (called from Request.read()).