Search before asking
Motivation
IoUtils.toByteArray(InputStream) in fesod-common currently looks like this:
public static byte[] toByteArray(final InputStream input) throws IOException {
final ByteArrayOutputStream output = new ByteArrayOutputStream();
try {
copy(input, output);
return output.toByteArray();
} finally {
output.toByteArray();
}
}
The finally block calls output.toByteArray() and discards the result. ByteArrayOutputStream.toByteArray() allocates a complete copy of the internal buffer, so every call to this method allocates the full data twice.
This method is on real hot paths: WriteWorkbookHolder uses it to read entire template files into memory, and InputStreamImageConverter uses it for image cells. For a large template or image, the transient extra allocation equals the whole payload size (e.g. an extra ~100 MB for a 100 MB template), for no effect.
It appears to be a leftover from translating commons-io's try-with-resources version (IOUtils.toByteArray), where the finally would have been output.close() - a no-op for ByteArrayOutputStream.
Solution
Remove the try/finally entirely, since ByteArrayOutputStream does not need closing:
public static byte[] toByteArray(final InputStream input) throws IOException {
final ByteArrayOutputStream output = new ByteArrayOutputStream();
copy(input, output);
return output.toByteArray();
}
No behavior change - the existing IoUtilsTest cases continue to pass unchanged.
Alternatives
Keep the structure but mirror commons-io with try-with-resources
(try (ByteArrayOutputStream output = new ByteArrayOutputStream())). Functionally
equivalent, since ByteArrayOutputStream.close() is a no-op.
Anything else?
The method is inherited from the EasyExcel lineage; upstream commons-io's
IOUtils.toByteArray uses try-with-resources, which supports the reading that
the finally block was meant to be a close() call.
Are you willing to submit a PR?
Search before asking
Motivation
IoUtils.toByteArray(InputStream)infesod-commoncurrently looks like this:The
finallyblock callsoutput.toByteArray()and discards the result.ByteArrayOutputStream.toByteArray()allocates a complete copy of the internal buffer, so every call to this method allocates the full data twice.This method is on real hot paths:
WriteWorkbookHolderuses it to read entire template files into memory, andInputStreamImageConverteruses it for image cells. For a large template or image, the transient extra allocation equals the whole payload size (e.g. an extra ~100 MB for a 100 MB template), for no effect.It appears to be a leftover from translating commons-io's try-with-resources version (
IOUtils.toByteArray), where thefinallywould have beenoutput.close()- a no-op forByteArrayOutputStream.Solution
Remove the
try/finallyentirely, sinceByteArrayOutputStreamdoes not need closing:No behavior change - the existing
IoUtilsTestcases continue to pass unchanged.Alternatives
Keep the structure but mirror commons-io with try-with-resources
(
try (ByteArrayOutputStream output = new ByteArrayOutputStream())). Functionallyequivalent, since
ByteArrayOutputStream.close()is a no-op.Anything else?
The method is inherited from the EasyExcel lineage; upstream commons-io's
IOUtils.toByteArrayuses try-with-resources, which supports the reading thatthe
finallyblock was meant to be aclose()call.Are you willing to submit a PR?