mbeckerle commented on code in PR #1603:
URL: https://github.com/apache/daffodil/pull/1603#discussion_r2628534863
##########
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:
Ok. Thanks for double checking that the reuse is at least working. When we
call parse repeatedly in a loop such as to parse a stream of messages, is that
going to allocate this over and over, or are we reusing the PState in that
case.
I answered my own question... we have to reuse the Pstate because the I/O
position on the input has to be preserved.
However, other applications will call parse once per input record because
they are carving off the input data record/message themselves based on UDP
datagram size or other means. In that case those are brand new calls to parse
with new PState allocated. This is also a fairly tight loop, so yes,
eliminating this 1MB allocation and GC for each parse call should be an
improvement.
We should consider pooling entire PState objects and resetting them so that
we don't have to reallocate any of this stuff.
--
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]
