-
Notifications
You must be signed in to change notification settings - Fork 38.4k
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
Reduce byte array allocations in StompEncoder #24694
Reduce byte array allocations in StompEncoder #24694
Conversation
Good suggestion. Couldn't we achieve that with |
I like the idea of not having to add/use an additional external dependency, but I'm not sure if a ByteBuffer would help here.
Using
Since ByteBuffers are fixed-sized, we'd have the same allocation and growth problems as before. |
Yes I meant a heap buffer. A
This is saying there is still one allocation at least. I don't know quite follow, what other arrays are needed here that would be taken from a pool? |
In our case (because our stomp-headers are bigger than 128 byte), the current implementation allocates 3 byte arrays for each stomp message we send:
(The numbers don't add up correctly because the profiler I used doesn't track every single allocation as explained here: https://github.com/jvm-profiling-tools/async-profiler#allocation-profiling ) When sending 10000 messages, this makes 30000 byte arrays When using
How would that work, if the the underlying byte array is too small, or too big? |
Thanks for elaborating. So we are doing a very poor job of estimating the required size. We have all the input at hand. We should be able to estimate more precisely, but turning the Come to think of it we also have https://docs.spring.io/spring/docs/current/javadoc-api/org/springframework/util/FastByteArrayOutputStream.html which should be able to do the same. |
I did a quick check for our application: We'd need a initial buffer of approx.
|
Something like this rstoyanchev@fa220ff, basically just aggregating and deferring allocation until the end. That eliminates resizing. I'm also wondering if we can also avoid the array copy when taking |
The commit you've linked looks very promising. I've left some inline comments. |
60bc7dd
to
37dd84c
Compare
The use of an okio.Buffer instead of an ByteArrayOutputStream will reduce the amount of allocated byte arrays by 50% to 66%. With the current ByteArrayOutputStream-based implementation 2-3 byte arrays are allocated. One when the stream is created, one when toByteArray() is called and maybe a third one, when the stream must grow because the overhead(headers) exeeds 128 bytes. With an okio.Buffer, only one byte array has to be allocated, when readByteArray() is called. All other needed arrays will be taken from Okio's internal Segment Pool. The pattern of conditionally using classes, when they are present on the classpath is inspired by RestTemplate.
37dd84c
to
ed92de3
Compare
I've incorporated your feedback, thanks. |
Thanks for fixing this so fast 👍 Will this only be part of 5.3 M1 or also back-ported to 5.2.x? |
This is in master and we haven't branched for 5.3 yet, so it will be in 5.2.6. |
The use of an
okio.Buffer
instead of anByteArrayOutputStream
will reducethe amount of allocated byte arrays by 50% to 66%.
With the current
ByteArrayOutputStream
-based implementation, 2-3 byte arraysare allocated. One when the stream is created, one when
toByteArray()
is calledand maybe a third one, when the stream must grow because the overhead(headers) exeeds 128 bytes.
With an
okio.Buffer
, only one byte array has to be allocated, whenreadByteArray()
is called.All other needed arrays will be taken from Okio's internal Segment Pool.
The pattern of conditionally using classes, when they are present on the classpath
is inspired by RestTemplate.
I've choosen this older okio version, because its the same version that would be pulled by the already managed okhttp version.
Context
While profiling our application we noticed that a considerable amount of byte arrays is created in

org.springframework.messaging.simp.stomp.StompEncoder.encode
:In other parts of our application, we already made good experience with using
okio.Buffer
s instead ofByteArrayOutputStream
s