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

Improve performance of StompEncoder.encode() [SPR-14747] #19313

Closed
spring-projects-issues opened this issue Sep 25, 2016 · 2 comments
Closed

Improve performance of StompEncoder.encode() [SPR-14747] #19313

spring-projects-issues opened this issue Sep 25, 2016 · 2 comments

Comments

@spring-projects-issues
Copy link
Collaborator

@spring-projects-issues spring-projects-issues commented Sep 25, 2016

Christoph Dreis opened SPR-14747 and commented

Hey,

I just noticed a rather costly assertion in the websocket layer - more explicitly in StompEncoder.encode(). Sorry I didn't notice this earlier in order to bring this to 4.3.3.

It is very similar to #19191, but has a much bigger impact on websocket applications as its executed on basically every message apart from heartbeats. Apart from the JMH benchmarks below, which show a factor of ~1200, the Assert statement in question produced more than 60GB of heap pressure in just 20 seconds. Mostly coming from the string concatenation of course.

!stomp-encoder.jpg|thumbnail!

Benchmark Mode Cnt Score Error Units
MyBenchmark.testNormal thrpt 100 1639083,456 ± 68418,140 ops/s
MyBenchmark.testEnhanced thrpt 100 2087033509,372 ± 21891731,313 ops/s

Much like the fix for #19191 I switched from Assert.notNull to a simple null check with an IllegalStateException.

While looking into StompEncoder I also moved one variable declaration a bit down, which was not needed in every-case. I hope you don't mind.

Best,
Christoph


Affects: 4.2.8, 4.3.3

Reference URL: #1185

Attachments:

Issue Links:

  • #19191 Improve performance of assertion in StompSubProtocolHandler

Referenced from: commits f2e1e1b, 6c764f6, 02d83ce, 94753b5, a6b0b6e, 774e4c3, 6577faa

Backported to: 4.2.9

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Sep 25, 2016

Juergen Hoeller commented

Once again, a good catch! Merged now, to be backported to 4.3.4 and 4.2.9 tomorrow.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Sep 26, 2016

Christoph Dreis commented

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants