allocation free AdaptiveByteBuf::setBytes(byte[])#16058
Conversation
Motivation: The adaptive buffer's setBytes(byte[]) method requires the underlying internal NIO buffer to be materialized, but temporary buffers (which won't get flushed to the network) are not granted to have it, causing an unwanted allocation Modification: Java 13 expose ad-hoc method which allow using the root parent NIO buffer, saving materializing the internal NIO buffer. Result: Cheaper setBytes
|
This has popped out on some benchmark 
And it's not the only place. More patches will arrive soon(ish). |
| if (PlatformDependent0.hasAbsolutePutArrayMethod()) { | ||
| return PlatformDependent0.absolutePut(dst, dstOffset, src, srcOffset, length); | ||
| } else { | ||
| ByteBuffer tmp = (ByteBuffer) dst.duplicate().clear().position(dstOffset).limit(dstOffset + length); |
There was a problem hiding this comment.
This part is not tested
| checkIndex(index, length); | ||
| ByteBuffer tmp = (ByteBuffer) internalNioBuffer().clear().position(index); | ||
| tmp.put(src, srcIndex, length); | ||
| if (tmpNioBuf == null && PlatformDependent.javaVersion() >= 13) { |
There was a problem hiding this comment.
hasAbsolutePutArrayMethod instead of javaVersion, maybe?
Motivation: The adaptive buffer's setBytes(byte[]) method requires the underlying internal NIO buffer to be materialized, but temporary buffers (which won't get flushed to the network) are not granted to have it, causing an unwanted allocation Modification: Java 13 expose ad-hoc method which allow using the root parent NIO buffer, saving materializing the internal NIO buffer. Result: Cheaper setBytes (cherry picked from commit 3cae6f6)
chrisvest
added
the
needs-cherry-pick-5.0
|
Hi @franz1981, if I may ask regarding
Could it be that those unwanted allocations happen on heap or it's always materialized into direct buffers? I was recently getting on-heap OOM when using direct buffers and saw Copying only relevant for this PR part from the question |
|
@BaurzhanSakhariev it is possible but I would still be surprised unless there is some leak on the source NIO heap buffer (that's why the copy happen looking at the stack trace) which keep it referenced and prevent it to be GCd. This PR is not fixing the src side but the designation side (which receive the copy) so it is possible you were already full w the heap and the additional allocation on dst side just give the final blow |
Right, I run test with low However, that was not a good idea to cap it like that as I run tests on same JVM with my application and created many structures in test itself and had very small max heap setting. Once I start running tests with "normal" max heap and instead directly specified |
Motivation: The adaptive buffer's setBytes(byte[]) method requires the underlying internal NIO buffer to be materialized, but temporary buffers (which won't get flushed to the network) are not granted to have it, causing an unwanted allocation Modification: Java 13 expose ad-hoc method which allow using the root parent NIO buffer, saving materializing the internal NIO buffer. Result: Cheaper setBytes (cherry picked from commit 3cae6f6) Co-authored-by: Francesco Nigro <[email protected]>
5 participants
Motivation:
The adaptive buffer's setBytes(byte[]) method requires the underlying internal NIO buffer to be materialized, but temporary buffers (which won't get flushed to the network) are not granted to have it, causing an unwanted allocation
Modification:
Java 13 expose ad-hoc method which allow using the root parent NIO buffer, saving materializing the internal NIO buffer.
Result:
Cheaper setBytes