Author: gwoolsey
Date: Wed Dec 6 00:15:51 2017
New Revision: 1817252
URL: http://svn.apache.org/viewvc?rev=1817252&view=rev
Log:
Bug #61841 - Unnecessary long computation when evaluating VLOOKUP on all column
reference
Found some optimizations in the general evaluation framework related to blank
cells in rows beyond the last defined row of a sheet.
I don't see any issue with passing a bit of context down deeper into this
framework, as it's all POI-internal and only had one calling path.
See the above bug for the performance analysis. Not specifically related to
VLOOKUP, but improves that case by more than 2/3 as well.
Added:
poi/trunk/src/ooxml/testcases/org/apache/poi/ss/formula/functions/TestVlookup.java
(with props)
poi/trunk/test-data/spreadsheet/VLookupFullColumn.xlsx (with props)
Modified:
poi/trunk/src/java/org/apache/poi/hssf/usermodel/HSSFEvaluationSheet.java
poi/trunk/src/java/org/apache/poi/ss/formula/CellEvaluationFrame.java
poi/trunk/src/java/org/apache/poi/ss/formula/EvaluationSheet.java
poi/trunk/src/java/org/apache/poi/ss/formula/EvaluationTracker.java
poi/trunk/src/java/org/apache/poi/ss/formula/FormulaUsedBlankCellSet.java
poi/trunk/src/java/org/apache/poi/ss/formula/WorkbookEvaluator.java
poi/trunk/src/java/org/apache/poi/ss/formula/eval/forked/ForkedEvaluationSheet.java
poi/trunk/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFEvaluationSheet.java
poi/trunk/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFEvaluationSheet.java
Modified:
poi/trunk/src/java/org/apache/poi/hssf/usermodel/HSSFEvaluationSheet.java
URL:
http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/hssf/usermodel/HSSFEvaluationSheet.java?rev=1817252&r1=1817251&r2=1817252&view=diff
==============================================================================
--- poi/trunk/src/java/org/apache/poi/hssf/usermodel/HSSFEvaluationSheet.java
(original)
+++ poi/trunk/src/java/org/apache/poi/hssf/usermodel/HSSFEvaluationSheet.java
Wed Dec 6 00:15:51 2017
@@ -28,14 +28,25 @@ import org.apache.poi.util.Internal;
final class HSSFEvaluationSheet implements EvaluationSheet {
private final HSSFSheet _hs;
+ private int _lastDefinedRow = -1;
public HSSFEvaluationSheet(HSSFSheet hs) {
_hs = hs;
+ _lastDefinedRow = _hs.getLastRowNum();
}
public HSSFSheet getHSSFSheet() {
return _hs;
}
+
+ /* (non-Javadoc)
+ * @see org.apache.poi.ss.formula.EvaluationSheet#getlastRowNum()
+ * @since POI 4.0.0
+ */
+ public int getlastRowNum() {
+ return _lastDefinedRow;
+ }
+
@Override
public EvaluationCell getCell(int rowIndex, int columnIndex) {
HSSFRow row = _hs.getRow(rowIndex);
@@ -54,6 +65,6 @@ final class HSSFEvaluationSheet implemen
*/
@Override
public void clearAllCachedResultValues() {
- // nothing to do
+ _lastDefinedRow = _hs.getLastRowNum();
}
}
Modified: poi/trunk/src/java/org/apache/poi/ss/formula/CellEvaluationFrame.java
URL:
http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/ss/formula/CellEvaluationFrame.java?rev=1817252&r1=1817251&r2=1817252&view=diff
==============================================================================
--- poi/trunk/src/java/org/apache/poi/ss/formula/CellEvaluationFrame.java
(original)
+++ poi/trunk/src/java/org/apache/poi/ss/formula/CellEvaluationFrame.java Wed
Dec 6 00:15:51 2017
@@ -64,11 +64,11 @@ final class CellEvaluationFrame {
_sensitiveInputCells.toArray(result);
return result;
}
- public void addUsedBlankCell(int bookIndex, int sheetIndex, int
rowIndex, int columnIndex) {
+ public void addUsedBlankCell(EvaluationWorkbook evalWorkbook, int
bookIndex, int sheetIndex, int rowIndex, int columnIndex) {
if (_usedBlankCellGroup == null) {
_usedBlankCellGroup = new FormulaUsedBlankCellSet();
}
- _usedBlankCellGroup.addCell(bookIndex, sheetIndex, rowIndex,
columnIndex);
+ _usedBlankCellGroup.addCell(evalWorkbook, bookIndex,
sheetIndex, rowIndex, columnIndex);
}
public void updateFormulaResult(ValueEval result) {
Modified: poi/trunk/src/java/org/apache/poi/ss/formula/EvaluationSheet.java
URL:
http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/ss/formula/EvaluationSheet.java?rev=1817252&r1=1817251&r2=1817252&view=diff
==============================================================================
--- poi/trunk/src/java/org/apache/poi/ss/formula/EvaluationSheet.java (original)
+++ poi/trunk/src/java/org/apache/poi/ss/formula/EvaluationSheet.java Wed Dec
6 00:15:51 2017
@@ -17,6 +17,7 @@
package org.apache.poi.ss.formula;
+import org.apache.poi.ss.usermodel.Sheet;
import org.apache.poi.util.Internal;
/**
@@ -42,4 +43,10 @@ public interface EvaluationSheet {
* @since POI 3.15 beta 3
*/
public void clearAllCachedResultValues();
+
+ /**
+ * @return last row index referenced on this sheet, for evaluation
optimization
+ * @since POI 4.0.0
+ */
+ public int getlastRowNum();
}
Modified: poi/trunk/src/java/org/apache/poi/ss/formula/EvaluationTracker.java
URL:
http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/ss/formula/EvaluationTracker.java?rev=1817252&r1=1817251&r2=1817252&view=diff
==============================================================================
--- poi/trunk/src/java/org/apache/poi/ss/formula/EvaluationTracker.java
(original)
+++ poi/trunk/src/java/org/apache/poi/ss/formula/EvaluationTracker.java Wed Dec
6 00:15:51 2017
@@ -131,7 +131,7 @@ final class EvaluationTracker {
}
}
- public void acceptPlainValueDependency(int bookIndex, int sheetIndex,
+ public void acceptPlainValueDependency(EvaluationWorkbook evalWorkbook,
int bookIndex, int sheetIndex,
int rowIndex, int columnIndex, ValueEval value) {
// Tell the currently evaluating cell frame that it has a
dependency on the specified
int prevFrameIndex = _evaluationFrames.size() - 1;
@@ -140,7 +140,7 @@ final class EvaluationTracker {
} else {
CellEvaluationFrame consumingFrame =
_evaluationFrames.get(prevFrameIndex);
if (value == BlankEval.instance) {
- consumingFrame.addUsedBlankCell(bookIndex,
sheetIndex, rowIndex, columnIndex);
+ consumingFrame.addUsedBlankCell(evalWorkbook,
bookIndex, sheetIndex, rowIndex, columnIndex);
} else {
PlainValueCellCacheEntry cce =
_cache.getPlainValueEntry(bookIndex, sheetIndex,
rowIndex, columnIndex, value);
Modified:
poi/trunk/src/java/org/apache/poi/ss/formula/FormulaUsedBlankCellSet.java
URL:
http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/ss/formula/FormulaUsedBlankCellSet.java?rev=1817252&r1=1817251&r2=1817252&view=diff
==============================================================================
--- poi/trunk/src/java/org/apache/poi/ss/formula/FormulaUsedBlankCellSet.java
(original)
+++ poi/trunk/src/java/org/apache/poi/ss/formula/FormulaUsedBlankCellSet.java
Wed Dec 6 00:15:51 2017
@@ -57,13 +57,17 @@ final class FormulaUsedBlankCellSet {
private int _firstColumnIndex;
private int _lastColumnIndex;
private BlankCellRectangleGroup _currentRectangleGroup;
+ private int _lastDefinedRow;
- public BlankCellSheetGroup() {
+ public BlankCellSheetGroup(int lastDefinedRow) {
_rectangleGroups = new ArrayList<>();
_currentRowIndex = -1;
+ _lastDefinedRow = lastDefinedRow;
}
public void addCell(int rowIndex, int columnIndex) {
+ if (rowIndex > _lastDefinedRow) return;
+
if (_currentRowIndex == -1) {
_currentRowIndex = rowIndex;
_firstColumnIndex = columnIndex;
@@ -89,6 +93,8 @@ final class FormulaUsedBlankCellSet {
}
public boolean containsCell(int rowIndex, int columnIndex) {
+ if (rowIndex > _lastDefinedRow) return true;
+
for (int i=_rectangleGroups.size()-1; i>=0; i--) {
BlankCellRectangleGroup bcrg =
_rectangleGroups.get(i);
if (bcrg.containsCell(rowIndex, columnIndex)) {
@@ -167,17 +173,17 @@ final class FormulaUsedBlankCellSet {
_sheetGroupsByBookSheet = new HashMap<>();
}
- public void addCell(int bookIndex, int sheetIndex, int rowIndex, int
columnIndex) {
- BlankCellSheetGroup sbcg = getSheetGroup(bookIndex, sheetIndex);
+ public void addCell(EvaluationWorkbook evalWorkbook, int bookIndex, int
sheetIndex, int rowIndex, int columnIndex) {
+ BlankCellSheetGroup sbcg = getSheetGroup(evalWorkbook,
bookIndex, sheetIndex);
sbcg.addCell(rowIndex, columnIndex);
}
- private BlankCellSheetGroup getSheetGroup(int bookIndex, int
sheetIndex) {
+ private BlankCellSheetGroup getSheetGroup(EvaluationWorkbook
evalWorkbook, int bookIndex, int sheetIndex) {
BookSheetKey key = new BookSheetKey(bookIndex, sheetIndex);
BlankCellSheetGroup result = _sheetGroupsByBookSheet.get(key);
if (result == null) {
- result = new BlankCellSheetGroup();
+ result = new
BlankCellSheetGroup(evalWorkbook.getSheet(sheetIndex).getlastRowNum());
_sheetGroupsByBookSheet.put(key, result);
}
return result;
Modified: poi/trunk/src/java/org/apache/poi/ss/formula/WorkbookEvaluator.java
URL:
http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/ss/formula/WorkbookEvaluator.java?rev=1817252&r1=1817251&r2=1817252&view=diff
==============================================================================
--- poi/trunk/src/java/org/apache/poi/ss/formula/WorkbookEvaluator.java
(original)
+++ poi/trunk/src/java/org/apache/poi/ss/formula/WorkbookEvaluator.java Wed Dec
6 00:15:51 2017
@@ -253,7 +253,7 @@ public final class WorkbookEvaluator {
if (srcCell == null || srcCell.getCellType() != CellType.FORMULA) {
ValueEval result = getValueFromNonFormulaCell(srcCell);
if (shouldCellDependencyBeRecorded) {
- tracker.acceptPlainValueDependency(_workbookIx, sheetIndex,
rowIndex, columnIndex, result);
+ tracker.acceptPlainValueDependency(_workbook, _workbookIx,
sheetIndex, rowIndex, columnIndex, result);
}
return result;
}
Modified:
poi/trunk/src/java/org/apache/poi/ss/formula/eval/forked/ForkedEvaluationSheet.java
URL:
http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/ss/formula/eval/forked/ForkedEvaluationSheet.java?rev=1817252&r1=1817251&r2=1817252&view=diff
==============================================================================
---
poi/trunk/src/java/org/apache/poi/ss/formula/eval/forked/ForkedEvaluationSheet.java
(original)
+++
poi/trunk/src/java/org/apache/poi/ss/formula/eval/forked/ForkedEvaluationSheet.java
Wed Dec 6 00:15:51 2017
@@ -42,6 +42,7 @@ import org.apache.poi.util.Internal;
final class ForkedEvaluationSheet implements EvaluationSheet {
private final EvaluationSheet _masterSheet;
+
/**
* Only cells which have been split are put in this map. (This has been
done to conserve memory).
*/
@@ -51,7 +52,15 @@ final class ForkedEvaluationSheet implem
_masterSheet = masterSheet;
_sharedCellsByRowCol = new HashMap<>();
}
-
+
+ /* (non-Javadoc)
+ * @see org.apache.poi.ss.formula.EvaluationSheet#getlastRowNum()
+ * @since POI 4.0.0
+ */
+ public int getlastRowNum() {
+ return _masterSheet.getlastRowNum();
+ }
+
@Override
public EvaluationCell getCell(int rowIndex, int columnIndex) {
RowColKey key = new RowColKey(rowIndex, columnIndex);
Modified:
poi/trunk/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFEvaluationSheet.java
URL:
http://svn.apache.org/viewvc/poi/trunk/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFEvaluationSheet.java?rev=1817252&r1=1817251&r2=1817252&view=diff
==============================================================================
---
poi/trunk/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFEvaluationSheet.java
(original)
+++
poi/trunk/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFEvaluationSheet.java
Wed Dec 6 00:15:51 2017
@@ -27,14 +27,25 @@ import org.apache.poi.util.Internal;
@Internal
final class SXSSFEvaluationSheet implements EvaluationSheet {
private final SXSSFSheet _xs;
+ private int _lastDefinedRow = -1;
public SXSSFEvaluationSheet(SXSSFSheet sheet) {
_xs = sheet;
+ _lastDefinedRow = _xs.getLastRowNum();
}
public SXSSFSheet getSXSSFSheet() {
return _xs;
}
+
+ /* (non-Javadoc)
+ * @see org.apache.poi.ss.formula.EvaluationSheet#getlastRowNum()
+ * @since POI 4.0.0
+ */
+ public int getlastRowNum() {
+ return _lastDefinedRow;
+ }
+
@Override
public EvaluationCell getCell(int rowIndex, int columnIndex) {
SXSSFRow row = _xs.getRow(rowIndex);
@@ -56,6 +67,6 @@ final class SXSSFEvaluationSheet impleme
*/
@Override
public void clearAllCachedResultValues() {
- // nothing to do
+ _lastDefinedRow = _xs.getLastRowNum();
}
}
Modified:
poi/trunk/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFEvaluationSheet.java
URL:
http://svn.apache.org/viewvc/poi/trunk/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFEvaluationSheet.java?rev=1817252&r1=1817251&r2=1817252&view=diff
==============================================================================
---
poi/trunk/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFEvaluationSheet.java
(original)
+++
poi/trunk/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFEvaluationSheet.java
Wed Dec 6 00:15:51 2017
@@ -34,25 +34,40 @@ final class XSSFEvaluationSheet implemen
private final XSSFSheet _xs;
private Map<CellKey, EvaluationCell> _cellCache;
+ private int _lastDefinedRow = -1;
public XSSFEvaluationSheet(XSSFSheet sheet) {
_xs = sheet;
+ _lastDefinedRow = _xs.getLastRowNum();
}
public XSSFSheet getXSSFSheet() {
return _xs;
}
+ /* (non-Javadoc)
+ * @see org.apache.poi.ss.formula.EvaluationSheet#getlastRowNum()
+ * @since POI 4.0.0
+ */
+ public int getlastRowNum() {
+ return _lastDefinedRow;
+ }
+
/* (non-JavaDoc), inherit JavaDoc from EvaluationWorkbook
* @since POI 3.15 beta 3
*/
@Override
public void clearAllCachedResultValues() {
_cellCache = null;
+ _lastDefinedRow = _xs.getLastRowNum();
}
@Override
public EvaluationCell getCell(int rowIndex, int columnIndex) {
+ // shortcut evaluation if reference is outside the bounds of existing
data
+ // see issue #61841 for impact on VLOOKUP in particular
+ if (rowIndex > _lastDefinedRow) return null;
+
// cache for performance: ~30% speedup due to caching
if (_cellCache == null) {
_cellCache = new HashMap<>(_xs.getLastRowNum() * 3);
Added:
poi/trunk/src/ooxml/testcases/org/apache/poi/ss/formula/functions/TestVlookup.java
URL:
http://svn.apache.org/viewvc/poi/trunk/src/ooxml/testcases/org/apache/poi/ss/formula/functions/TestVlookup.java?rev=1817252&view=auto
==============================================================================
---
poi/trunk/src/ooxml/testcases/org/apache/poi/ss/formula/functions/TestVlookup.java
(added)
+++
poi/trunk/src/ooxml/testcases/org/apache/poi/ss/formula/functions/TestVlookup.java
Wed Dec 6 00:15:51 2017
@@ -0,0 +1,25 @@
+package org.apache.poi.ss.formula.functions;
+
+import org.apache.poi.ss.usermodel.CellType;
+import org.apache.poi.ss.usermodel.FormulaEvaluator;
+import org.apache.poi.ss.usermodel.Workbook;
+import org.apache.poi.xssf.XSSFTestDataSamples;
+import org.junit.Test;
+
+import junit.framework.TestCase;
+
+/**
+ * Test the VLOOKUP function
+ */
+public class TestVlookup extends TestCase {
+
+ @Test
+ public void testFullColumnAreaRef61841() {
+ final Workbook wb =
XSSFTestDataSamples.openSampleWorkbook("VLookupFullColumn.xlsx");
+ FormulaEvaluator feval =
wb.getCreationHelper().createFormulaEvaluator();
+ feval.evaluateAll();
+ assertEquals("Wrong lookup value", "Value1",
feval.evaluate(wb.getSheetAt(0).getRow(3).getCell(1)).getStringValue());
+ assertEquals("Lookup should return #N/A", CellType.ERROR,
feval.evaluate(wb.getSheetAt(0).getRow(4).getCell(1)).getCellType());
+ }
+
+}
Propchange:
poi/trunk/src/ooxml/testcases/org/apache/poi/ss/formula/functions/TestVlookup.java
------------------------------------------------------------------------------
svn:eol-style = native
Added: poi/trunk/test-data/spreadsheet/VLookupFullColumn.xlsx
URL:
http://svn.apache.org/viewvc/poi/trunk/test-data/spreadsheet/VLookupFullColumn.xlsx?rev=1817252&view=auto
==============================================================================
Binary file - no diff available.
Propchange: poi/trunk/test-data/spreadsheet/VLookupFullColumn.xlsx
------------------------------------------------------------------------------
svn:mime-type = application/octet-stream
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
