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 readNBytes in ByteArrayHttpMessageConverter when contentLength is available #30010

Conversation

kilink
Copy link
Contributor

@kilink kilink commented Feb 22, 2023

If the content length is available, pass it to readNBytes when reading the message body. When the content length is less than the internal buffer size in InputStream (8192), this avoids a copy, as readNBytes will return the buffer directly. When the content length is greater than the buffer size used in InputStream, passing the content length at least avoids over-allocating the final buffer (e.g., if the content length were 8193 bytes, 1 byte more than the default buffer size).

If the content length isn't present or is too large to represent as an integer, fall back to the default behavior of readAllBytes by passing in Integer.MAX_VALUE.

If the content length is available, pass it to readNBytes when reading
the message body. When the content length is less than the internal
buffer size in InputStream (8192), this avoids a copy, as readNBytes
will return the buffer directly. When the content length is greater
than the buffer size used in InputStream, passing the content length
at least avoids over-allocating the final buffer (e.g., if the content
length were 8193 bytes, 1 byte more than the default buffer size).

If the content length isn't present or is too large to represent as an integer,
fall back to the default behavior of readAllBytes by passing in Integer.MAX_VALUE.
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Feb 22, 2023
Copy link
Contributor

@rstoyanchev rstoyanchev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the suggestion. I think we could do it in the main branch, since readNBytes was added in JDK 11.

One minor comment on the implementation. Given that readAllBytes internally is readNBytes(Integer.MAX_VALUE), couldn't the logic then be such that if there is a suitable contentLength call readNBytes, or otherwise fall back on readAllBytes.

@rstoyanchev rstoyanchev added in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Feb 24, 2023
@rstoyanchev rstoyanchev added this to the 6.0.6 milestone Feb 24, 2023
@rstoyanchev rstoyanchev requested a review from jhoeller February 24, 2023 17:05
@rstoyanchev rstoyanchev changed the title Use content length to size buffer in ByteArrayHttpMessageConverter Use readNBytes in ByteArrayHttpMessageConverter when when contentLength is available Feb 24, 2023
@rstoyanchev rstoyanchev changed the title Use readNBytes in ByteArrayHttpMessageConverter when when contentLength is available Use readNBytes in ByteArrayHttpMessageConverter when contentLength is available Feb 24, 2023
@rstoyanchev rstoyanchev self-assigned this Mar 2, 2023
rstoyanchev pushed a commit that referenced this pull request Mar 2, 2023
If content-length is available, pass it to readNBytes in
ByteArrayHttpMessageConverter. When the content length is less than
the internal buffer size in InputStream (8192), this avoids a copy,
as readNBytes will return the buffer directly. When the content length
is greater than the buffer size used in InputStream, passing the
content-length at least avoids over-allocating the final buffer (e.g.,
if the content length were 8193 bytes, 1 byte more than the default
buffer size).

If the content length isn't present or is too large to represent as
an integer, fall back to the default behavior of readAllBytes by
passing in Integer.MAX_VALUE.

See gh-30010
@kilink kilink deleted the readNbytes-ByteArrayHttpMessageConverter branch March 15, 2024 19:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants