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.

Sign up for GitHub

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

Jump to bottom

Drop unnecessary intermediate StringBuilder.toString() call in ServerSentEventHttpMessageReader #25024

Closed
wants to merge 1 commit into from
Closed

Drop unnecessary intermediate StringBuilder.toString() call in ServerSentEventHttpMessageReader #25024

wants to merge 1 commit into from

Conversation

stsypanov
Copy link
Contributor

Apart from simplification this transformation is likely to bring faster code which consumes less memory.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label May 7, 2020
@jhoeller jhoeller 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 May 7, 2020
@jhoeller jhoeller added this to the 5.2.7 milestone May 7, 2020
@jhoeller
Copy link
Contributor

jhoeller commented May 7, 2020 •
edited

This looks entirely valid, worth applying to 5.2.7 from my perspective. Maybe an opportunity to revisit related code for efficient StringBuilder usage?

@jhoeller jhoeller changed the title Drop unnecessary intermediate StringBuilder.toString() call Drop unnecessary intermediate StringBuilder.toString() call in ServerSentEventHttpMessageReader May 8, 2020
@jhoeller jhoeller self-assigned this May 18, 2020
@jhoeller
Copy link
Contributor

I've taken this a bit further by enforcing StringBuilder exposure throughout the encode/decode methods in both ServerSentEventHttpMessageReader and ServerSentEventHttpMessageWriter, in particular also for the event content, not just for the comment part. Thanks for raising this!

@jhoeller jhoeller closed this in 9631db7 May 18, 2020
@stsypanov stsypanov deleted the substr branch May 18, 2020 18:51
FelixFly pushed a commit to FelixFly/spring-framework that referenced this pull request Aug 16, 2020
@jhoeller
Direct exposure of StringBuilder instead of intermediate String
749ae7b 
Closes spring-projectsgh-25024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@jhoeller jhoeller

Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement
Projects
None yet
Milestone
5.2.7
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@stsypanov @jhoeller @spring-projects-issues