You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@fesod.apache.org by "Aias00 (via GitHub)" <gi...@apache.org> on 2026/10/20 12:35:08 UTC
[PR] fix: reduce memory overhead when loading templates [fesod]
Aias00 opened a new pull request, #1246:
URL: https://github.com/apache/fesod/pull/1246
## Purpose of the pull request
Related: #185
Reduce the extra heap required by `withTemplate(...).build()` without changing template fill behavior.
Previously Fesod copied an entire template into a byte array and retained it until writer finish,
alongside POI's parsed workbook. InputStream templates additionally incurred a growing
ByteArrayOutputStream buffer and its final array copy.
This addresses the redundant template-copy overhead, not fully streaming template filling.
POI's parsed template workbook still resides in memory, including with `inMemory(false)`.
## What's changed?
- Read file and stream templates through BufferedInputStream instead of materializing their bytes.
- Close the prepared stream during workbook initialization, including parser and callback failures.
Preserve `autoCloseStream(false)` for caller-owned streams using the existing commons-io dependency.
- Keep the closed template stream non-null because existing fill and append paths also use it as
a template-present flag.
- Clean up an initialized workbook and owned output stream if construction fails. This preserves
output cleanup when template I/O failures move from eager copying to POI initialization;
cleanup failures are suppressed rather than replacing the original failure.
- Add coverage for XLS/XLSX, file/stream templates, both in-memory modes, repeated fills with
row shifting, append writes, source ownership, parser/I/O/callback failures and suppressed errors.
- Document the remaining template-memory cost in Javadocs and the English/Chinese fill guides.
### Allocation check
A local JDK 17 probe measured allocations on the current thread during WriteWorkbookHolder
construction, using a 32 MiB generated stream or a 32 MiB file. It warmed up five times and took
the minimum of three measured runs. The generated source does not allocate a 32 MiB input array.
Baseline classes were built from `0045070c1556d824e45851327e719867ec89a9d6`.
| Template source | Before (bytes allocated) | After (bytes allocated) |
| --- | ---: | ---: |
| File | 33,559,968 | 13,720 |
| InputStream | 100,668,808 | 13,400 |
These numbers measure template preparation only, not total workbook heap or POI parsing.
### Validation
- The new 32 MiB regression test failed before the change: template preparation eagerly read
33,554,432 bytes instead of zero; it passes after the change.
- The I/O failure regression also failed before initialization cleanup was added: the owned output
stream was not closed; it passes with the cleanup in place.
- `./mvnw -B -pl fesod-common,fesod-shaded,fesod-sheet -Dtest.excludedGroups=fuzz package`:
1,795 tests passed (60 common + 1,735 sheet), with no failures or skips.
- Java 8: 35 targeted tests passed across TemplateStreamBufferingTest, WorkBookUtilTest and
WriteWorkbookHolderOutputStreamTest.
- `./mvnw -B -pl fesod-common,fesod-shaded,fesod-sheet spotless:check`
- `./mvnw -B -pl fesod-common,fesod-shaded,fesod-sheet -Dmaven.javadoc.skip=false javadoc:javadoc`
## Checklist
- [x] I have read the [Contributor Guide](https://fesod.apache.org/community/contribution/).
- [x] I have written the necessary doc or comment.
- [x] I have added the necessary unit tests and all cases have passed.
--
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: commits-unsubscribe@fesod.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fesod.apache.org
For additional commands, e-mail: commits-help@fesod.apache.org
Re: [PR] fix: reduce memory overhead when loading templates [fesod]
Posted by "skytin1004 (via GitHub)" <gi...@apache.org>.
skytin1004 commented on code in PR #1246:
URL: https://github.com/apache/fesod/pull/1246#discussion_r4343307734
##########
fesod-sheet/src/test/java/org/apache/fesod/sheet/write/TemplateStreamBufferingTest.java:
##########
@@ -0,0 +1,330 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.fesod.sheet.write;
+
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.FilterInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.file.Files;
+import java.util.Arrays;
+import java.util.Collections;
+import org.apache.fesod.sheet.ExcelWriter;
+import org.apache.fesod.sheet.FesodSheet;
+import org.apache.fesod.sheet.testkit.Tags;
+import org.apache.fesod.sheet.testkit.base.AbstractExcelTest;
+import org.apache.fesod.sheet.testkit.enums.ExcelFormat;
+import org.apache.fesod.sheet.testkit.params.ExcelFormatSource;
+import org.apache.fesod.sheet.testkit.params.FormatScope;
+import org.apache.fesod.sheet.write.builder.ExcelWriterBuilder;
+import org.apache.fesod.sheet.write.handler.WorkbookWriteHandler;
+import org.apache.fesod.sheet.write.metadata.WriteSheet;
+import org.apache.fesod.sheet.write.metadata.WriteWorkbook;
+import org.apache.fesod.sheet.write.metadata.fill.FillConfig;
+import org.apache.fesod.sheet.write.metadata.holder.WriteWorkbookHolder;
+import org.apache.poi.hssf.usermodel.HSSFWorkbook;
+import org.apache.poi.ss.usermodel.Workbook;
+import org.apache.poi.ss.usermodel.WorkbookFactory;
+import org.apache.poi.xssf.usermodel.XSSFWorkbook;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+
+@Tag(Tags.WRITE)
+class TemplateStreamBufferingTest extends AbstractExcelTest {
+
+ @ParameterizedTest
+ @ValueSource(booleans = {true, false})
+ void preparingLargeTemplateDoesNotReadTheEntireInput(boolean autoClose) throws Exception {
+ GeneratedInputStream source = new GeneratedInputStream(32 * 1024 * 1024);
+ WriteWorkbook configuration = new WriteWorkbook();
+ configuration.setOutputStream(new ByteArrayOutputStream());
+ configuration.setTemplateInputStream(source);
+ configuration.setAutoCloseStream(autoClose);
+ WriteWorkbookHolder holder = new WriteWorkbookHolder(configuration);
+ try (InputStream prepared = holder.getTempTemplateInputStream()) {
+ Assertions.assertEquals(0, source.bytesRead, "preparing a template must not materialize its bytes");
+ Assertions.assertEquals(0, prepared.read());
+ Assertions.assertTrue(source.bytesRead < 32 * 1024 * 1024, "the first read must use bounded buffering");
+ }
+ }
+
+ @ParameterizedTest
+ @ExcelFormatSource(FormatScope.BINARY)
+ void fileAndStreamTemplatesRetainFillBehaviorAndOwnership(ExcelFormat format) throws Exception {
+ File template = template(format);
+ byte[] original = Files.readAllBytes(template.toPath());
+ for (boolean inMemory : new boolean[] {false, true}) {
+ File fileOutput = createTempFile("file-template", format);
+ fill(FesodSheet.write(fileOutput).withTemplate(template).inMemory(inMemory));
+ assertFilled(fileOutput);
+
+ for (boolean autoClose : new boolean[] {true, false}) {
+ File streamOutput = createTempFile("stream-template", format);
+ try (TrackingInputStream source = new TrackingInputStream(Files.newInputStream(template.toPath()))) {
+ ExcelWriterBuilder builder = FesodSheet.write(streamOutput)
+ .withTemplate(source)
+ .autoCloseStream(autoClose)
+ .inMemory(inMemory);
+ try (ExcelWriter writer = builder.build()) {
+ Assertions.assertEquals(autoClose ? 1 : 0, source.closeCalls);
+ InputStream prepared =
+ writer.writeContext().writeWorkbookHolder().getTempTemplateInputStream();
+ Assertions.assertNotNull(prepared, "the closed stream still marks a template workbook");
+ Assertions.assertThrows(IOException.class, prepared::read);
+ fill(writer);
+ }
+ Assertions.assertEquals(autoClose ? 1 : 0, source.closeCalls);
+ if (!autoClose) {
+ Assertions.assertEquals(-1, source.read());
+ }
+ }
+ assertFilled(streamOutput);
Review Comment:
Hi @Aias00, thanks for working on this.
I reproduced one remaining issue on Windows. With a file output and `autoCloseStream(false)`, the caller-owned template stream stays open as intended, but the `FileOutputStream` created internally by Fesod also stays open. As a result, the generated XLS and XLSX files remain locked after `finish()`.
Could you separate template-input ownership from internally created output ownership, and add a regression assertion that the generated file can be deleted or moved after `finish()`? Once this is covered, I think this will be ready to merge. Thanks!
--
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: commits-unsubscribe@fesod.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fesod.apache.org
For additional commands, e-mail: commits-help@fesod.apache.org