stevedlawrence commented on code in PR #1603:
URL: https://github.com/apache/daffodil/pull/1603#discussion_r2627592073
##########
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:
Note, the particular LocalBuffer that was related to the 1MB specified
length string allocation is actually from the InputSourceDataInputStream, not
the PState. So technically it's only a single 1MB allocation per ISDIS created.
If you are streaming data and reusing the same ISDIS you'll only have have one
allocation. But if you create a new ISDIS per parse (which non-streaming use
cases will do), then you will effectively get one allocation per parse() call.
So this performance only improves relatively small files that hvae
specified length strings that for non-streaming data.
--
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]
