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

Introduce toString(Charset) in FastByteArrayOutputStream #31737

Conversation

kilink
Copy link
Contributor

@kilink kilink commented Dec 1, 2023

Add a toString overload that accepts a Charset to mirror the method that has been on ByteArrayOutputStream since JDK10; additionally, add a special case for when a single buffer is in use internally to avoid the need to resize.

Add a toString overload that accepts a Charset to mirror the method that has been
on ByteArrayOutputStream since JDK10; additionally, add a special case for when
a single buffer is in use internally to avoid the need to resize.
@kilink
Copy link
Contributor Author

kilink commented Dec 1, 2023

Note this is somewhat of a follow up to #31731 and #30709; ContentCachingRequestWrapper was updated to use FastByteArrayOutputStream, which didn't have the toString(Charset) overload that we were previously calling when it was just ByteArrayOutputStream; adding a similar method to FastByteArrayOutputStream allows for a nice optimization when there's only a single buffer, as we can pass the byte array + offset + len to the String constructor and avoid any copying.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Dec 1, 2023
@sbrannen sbrannen self-assigned this Dec 2, 2023
@sbrannen sbrannen changed the title Add toString overload that accepts Charset to FastByteArrayOutputStream Introduce toString(Charset) in FastByteArrayOutputStream Dec 2, 2023
@sbrannen sbrannen added the in: core Issues in core modules (aop, beans, core, context, expression) label Dec 2, 2023
@jhoeller jhoeller added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Dec 2, 2023
@jhoeller jhoeller added this to the 6.1.2 milestone Dec 2, 2023
* to be used to decode the {@code bytes}
* @return a String decoded from the buffer's contents
*/
public String toString(Charset charset) {
Copy link
Member

Choose a reason for hiding this comment

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

For future reference, please keep in mind that we do not indent Javadoc tag content and that we need to add a @since tag for new elements in the public API.

However, I'll address those issues when merging, so there's no need to update this PR.

Copy link
Member

Choose a reason for hiding this comment

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

In addition, references to instance fields must be prefixed with this..

See Spring Framework's Code Style documentation for details.

@sbrannen sbrannen closed this in 7cdacf3 Dec 2, 2023
sbrannen added a commit that referenced this pull request Dec 2, 2023
@sbrannen
Copy link
Member

sbrannen commented Dec 2, 2023

This has been merged into main in 7cdacf3 and revised in cd62dfe.

Thanks

bclozel added a commit that referenced this pull request Dec 13, 2023
This commit builds on top of changes made in gh-29775 and gh-31737.
Before this change, we would allocate several byte arrays even in cases
of known request size. This could decrease performance when getting the
cached content as it requires merging several arrays and data is not
colocated in memory.

This change ensures that we create a `FastByteArrayOutputStream`
instance with the known request size so that the first allocated segment
can contain the entire content.
If the request size is not know, we will default back on the default
allocation size for the `FastByteArrayOutputStream`.

Closes gh-31834
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants