stevedlawrence commented on code in PR #1603:
URL: https://github.com/apache/daffodil/pull/1603#discussion_r2627558222
##########
daffodil-core/src/main/scala/org/apache/daffodil/io/LocalBuffer.scala:
##########
@@ -33,7 +33,13 @@ abstract class LocalBuffer[T <: java.nio.Buffer] {
def getBuf(length: Long) = {
Assert.usage(length <= Int.MaxValue)
if (tempBuf.isEmpty || tempBuf.get.capacity < length) {
- tempBuf = Maybe(allocate(length.toInt))
+ // allocate a buffer that can store the required length, but with a
minimum size. The
+ // majority of LocalBuffers should be smaller than this minimum size and
so should avoid
+ // costly reallocations, while still being small enough that the JVM
should have no
+ // problem quickly allocating it
+ val minBufferSize = 1024
+ val allocationSize = math.max(length.toInt, minBufferSize)
+ tempBuf = Maybe(allocate(allocationSize))
Review Comment:
I didn't check if with vs without reuse changed performance, but I did add a
print statement every time we call this getBuf function printed if we have to
allocate a new buffer or not. Testing with NITF (which has lots of specified
length strings and some relatively small files available), it allocated a 1024
byte buffer once and then reused it about 100+ times with no additional
allocates. We might need additional testing to make sure it's working correctly
everywhere for other formats, or it might be worth investigating other reuse
(e.g. make LocalBuffer reuse among threads), but I think the `LocalBuffer`
reuse logic is working as intended.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
