Bug 58896 and 52834: Cache Sheet.getMergedRegions() as it seems to sometimes be the culprit for autosize taking a very long time.

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1874973 13f79535-47bb-0310-9956-ffa450edef68
This commit is contained in:
Dominik Stadler 2020-03-08 11:17:34 +00:00
parent d57edf14f6
commit a7d96dcb73
3 changed files with 64 additions and 12 deletions

View File

@ -23,6 +23,7 @@ import java.awt.font.TextLayout;
import java.awt.geom.AffineTransform; import java.awt.geom.AffineTransform;
import java.awt.geom.Rectangle2D; import java.awt.geom.Rectangle2D;
import java.text.AttributedString; import java.text.AttributedString;
import java.util.List;
import java.util.Locale; import java.util.Locale;
import java.util.Map; import java.util.Map;
@ -118,6 +119,26 @@ public class SheetUtil {
* @return the width in pixels or -1 if cell is empty * @return the width in pixels or -1 if cell is empty
*/ */
public static double getCellWidth(Cell cell, int defaultCharWidth, DataFormatter formatter, boolean useMergedCells) { public static double getCellWidth(Cell cell, int defaultCharWidth, DataFormatter formatter, boolean useMergedCells) {
List<CellRangeAddress> mergedRegions = cell.getSheet().getMergedRegions();
return getCellWidth(cell, defaultCharWidth, formatter, useMergedCells, mergedRegions);
}
/**
* Compute width of a single cell
*
* This method receives the list of merged regions as querying it from the cell/sheet
* is time-consuming and thus caching the list across cells speeds up certain operations
* considerably.
*
* @param cell the cell whose width is to be calculated
* @param defaultCharWidth the width of a single character
* @param formatter formatter used to prepare the text to be measured
* @param useMergedCells whether to use merged cells
* @param mergedRegions The list of merged regions as received via cell.getSheet().getMergedRegions()
* @return the width in pixels or -1 if cell is empty
*/
public static double getCellWidth(Cell cell, int defaultCharWidth, DataFormatter formatter, boolean useMergedCells,
List<CellRangeAddress> mergedRegions) {
Sheet sheet = cell.getSheet(); Sheet sheet = cell.getSheet();
Workbook wb = sheet.getWorkbook(); Workbook wb = sheet.getWorkbook();
Row row = cell.getRow(); Row row = cell.getRow();
@ -126,7 +147,7 @@ public class SheetUtil {
// FIXME: this looks very similar to getCellWithMerges below. Consider consolidating. // FIXME: this looks very similar to getCellWithMerges below. Consider consolidating.
// We should only be checking merged regions if useMergedCells is true. Why are we doing this for-loop? // We should only be checking merged regions if useMergedCells is true. Why are we doing this for-loop?
int colspan = 1; int colspan = 1;
for (CellRangeAddress region : sheet.getMergedRegions()) { for (CellRangeAddress region : mergedRegions) {
if (region.isInRange(row.getRowNum(), column)) { if (region.isInRange(row.getRowNum(), column)) {
if (!useMergedCells) { if (!useMergedCells) {
// If we're not using merged cells, skip this one and move on to the next. // If we're not using merged cells, skip this one and move on to the next.
@ -247,11 +268,12 @@ public class SheetUtil {
DataFormatter formatter = new DataFormatter(); DataFormatter formatter = new DataFormatter();
int defaultCharWidth = getDefaultCharWidth(sheet.getWorkbook()); int defaultCharWidth = getDefaultCharWidth(sheet.getWorkbook());
List<CellRangeAddress> mergedRegions = sheet.getMergedRegions();
double width = -1; double width = -1;
for (int rowIdx = firstRow; rowIdx <= lastRow; ++rowIdx) { for (int rowIdx = firstRow; rowIdx <= lastRow; ++rowIdx) {
Row row = sheet.getRow(rowIdx); Row row = sheet.getRow(rowIdx);
if( row != null ) { if( row != null ) {
double cellWidth = getColumnWidthForRow(row, column, defaultCharWidth, formatter, useMergedCells); double cellWidth = getColumnWidthForRow(row, column, defaultCharWidth, formatter, useMergedCells, mergedRegions);
width = Math.max(width, cellWidth); width = Math.max(width, cellWidth);
} }
} }
@ -286,7 +308,8 @@ public class SheetUtil {
* @return the width in pixels or -1 if cell is empty * @return the width in pixels or -1 if cell is empty
*/ */
private static double getColumnWidthForRow( private static double getColumnWidthForRow(
Row row, int column, int defaultCharWidth, DataFormatter formatter, boolean useMergedCells) { Row row, int column, int defaultCharWidth, DataFormatter formatter, boolean useMergedCells,
List<CellRangeAddress> mergedRegions) {
if( row == null ) { if( row == null ) {
return -1; return -1;
} }
@ -297,7 +320,7 @@ public class SheetUtil {
return -1; return -1;
} }
return getCellWidth(cell, defaultCharWidth, formatter, useMergedCells); return getCellWidth(cell, defaultCharWidth, formatter, useMergedCells, mergedRegions);
} }
/** /**

View File

@ -36,6 +36,8 @@ import java.io.IOException;
import java.io.InputStream; import java.io.InputStream;
import java.io.OutputStream; import java.io.OutputStream;
import java.nio.charset.StandardCharsets; import java.nio.charset.StandardCharsets;
import java.time.Duration;
import java.time.Instant;
import java.util.Arrays; import java.util.Arrays;
import java.util.Calendar; import java.util.Calendar;
import java.util.HashMap; import java.util.HashMap;
@ -91,6 +93,8 @@ import org.apache.poi.ss.util.CellReference;
import org.apache.poi.ss.util.CellUtil; import org.apache.poi.ss.util.CellUtil;
import org.apache.poi.util.LocaleUtil; import org.apache.poi.util.LocaleUtil;
import org.apache.poi.util.NullOutputStream; import org.apache.poi.util.NullOutputStream;
import org.apache.poi.util.POILogFactory;
import org.apache.poi.util.POILogger;
import org.apache.poi.util.TempFile; import org.apache.poi.util.TempFile;
import org.apache.poi.util.XMLHelper; import org.apache.poi.util.XMLHelper;
import org.apache.poi.xssf.SXSSFITestDataProvider; import org.apache.poi.xssf.SXSSFITestDataProvider;
@ -116,6 +120,8 @@ import org.xml.sax.SAXParseException;
import org.xml.sax.XMLReader; import org.xml.sax.XMLReader;
public final class TestXSSFBugs extends BaseTestBugzillaIssues { public final class TestXSSFBugs extends BaseTestBugzillaIssues {
private static POILogger LOG = POILogFactory.getLogger(TestXSSFBugs.class);
public TestXSSFBugs() { public TestXSSFBugs() {
super(XSSFITestDataProvider.instance); super(XSSFITestDataProvider.instance);
} }
@ -3465,4 +3471,27 @@ public final class TestXSSFBugs extends BaseTestBugzillaIssues {
fail("Should catch exception as the file is corrupted"); fail("Should catch exception as the file is corrupted");
} }
} }
@Test
public void test58896WithFile() throws IOException {
try (Workbook wb = XSSFTestDataSamples.openSampleWorkbook("58896.xlsx")) {
Sheet sheet = wb.getSheetAt(0);
Instant start = Instant.now();
LOG.log(POILogger.INFO, "Autosizing columns...");
for (int i = 0; i < 3; ++i) {
LOG.log(POILogger.INFO, "Autosize " + i + " - " + Duration.between(start, Instant.now()));
sheet.autoSizeColumn(i);
}
for (int i = 0; i < 69 - 35 + 1; ++i)
for (int j = 0; j < 8; ++j) {
int col = 3 + 2 + i * (8 + 2) + j;
LOG.log(POILogger.INFO, "Autosize " + col + " - " + Duration.between(start, Instant.now()));
sheet.autoSizeColumn(col);
}
LOG.log(POILogger.INFO, Duration.between(start, Instant.now()));
}
}
} }

Binary file not shown.