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

Remove the un-required ByteBody split #964

Merged
merged 1 commit into from
Aug 15, 2024
Merged

Conversation

andriy-dmytruk
Copy link
Contributor

As @yawkat pointed out the BaseFilterProcessor uses byteBody().split(FASTEST) to get a body for request filters and leave an extra copy for actual request parsing. Therefore, assuming that we will only parse the body once, we do not need to copy the body when it is consumed by body binder. For the case of input streams and form data we only need to parse the body once, so we do that and cache the result in a multi-values field that can be reused.

Removing the split when it is not needed would prevent from unnecessary body caching.

As @yawkat pointed out the BaseFilterProcessor uses byteBody().split(FASTEST) to get a body for request filters and leave an extra copy for actual request parsing. Therefore, assuming that we will only parse the body once, we do not need to copy the body when it is consumed by body binder. For the case of input streams and form data we only need to parse the body once, so we do that and cache the result in a multi-values field that can be reused.

Removing the split when it is not needed would prevent from unnecessary body caching.
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
58.3% Coverage on New Code (required ≥ 70%)
1 New Critical Issues (required ≤ 0)

See analysis details on SonarCloud

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

@yawkat
Copy link
Member

yawkat commented Aug 15, 2024

Thanks!

@yawkat yawkat merged commit 365fd6f into 4.2.x Aug 15, 2024
20 of 21 checks passed
@yawkat yawkat deleted the andriy/byte-body-improve branch August 15, 2024 08:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants