From: Josh Micich Date: Thu, 4 Sep 2008 23:16:15 +0000 (+0000) Subject: Fix for bug 45376 - added caching to HSSFFormulaEvaluator X-Git-Tag: REL_3_2_FINAL~93 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=bae56b7be7a1c03c44a3f7f2918e156172e9bccf;p=poi.git Fix for bug 45376 - added caching to HSSFFormulaEvaluator git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@692300 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/src/java/org/apache/poi/hssf/record/formula/eval/LazyAreaEval.java b/src/java/org/apache/poi/hssf/record/formula/eval/LazyAreaEval.java index e43a97e405..e2dc42e056 100644 --- a/src/java/org/apache/poi/hssf/record/formula/eval/LazyAreaEval.java +++ b/src/java/org/apache/poi/hssf/record/formula/eval/LazyAreaEval.java @@ -23,7 +23,6 @@ import org.apache.poi.hssf.usermodel.HSSFCell; import org.apache.poi.hssf.usermodel.HSSFFormulaEvaluator; import org.apache.poi.hssf.usermodel.HSSFRow; import org.apache.poi.hssf.usermodel.HSSFSheet; -import org.apache.poi.hssf.usermodel.HSSFWorkbook; import org.apache.poi.hssf.util.CellReference; /** @@ -33,12 +32,12 @@ import org.apache.poi.hssf.util.CellReference; public final class LazyAreaEval extends AreaEvalBase { private final HSSFSheet _sheet; - private HSSFWorkbook _workbook; + private HSSFFormulaEvaluator _evaluator; - public LazyAreaEval(AreaI ptg, HSSFSheet sheet, HSSFWorkbook workbook) { + public LazyAreaEval(AreaI ptg, HSSFSheet sheet, HSSFFormulaEvaluator evaluator) { super(ptg); _sheet = sheet; - _workbook = workbook; + _evaluator = evaluator; } public ValueEval getRelativeValue(int relativeRowIndex, int relativeColumnIndex) { @@ -54,21 +53,21 @@ public final class LazyAreaEval extends AreaEvalBase { if (cell == null) { return BlankEval.INSTANCE; } - return HSSFFormulaEvaluator.getEvalForCell(cell, _sheet, _workbook); + return _evaluator.getEvalForCell(cell, _sheet); } public AreaEval offset(int relFirstRowIx, int relLastRowIx, int relFirstColIx, int relLastColIx) { AreaI area = new OffsetArea(getFirstRow(), getFirstColumn(), relFirstRowIx, relLastRowIx, relFirstColIx, relLastColIx); - return new LazyAreaEval(area, _sheet, _workbook); + return new LazyAreaEval(area, _sheet, _evaluator); } public String toString() { CellReference crA = new CellReference(getFirstRow(), getFirstColumn()); CellReference crB = new CellReference(getLastRow(), getLastColumn()); StringBuffer sb = new StringBuffer(); sb.append(getClass().getName()).append("["); - String sheetName = _workbook.getSheetName(_workbook.getSheetIndex(_sheet)); + String sheetName = _evaluator.getSheetName(_sheet); sb.append(sheetName); sb.append('!'); sb.append(crA.formatAsString()); diff --git a/src/java/org/apache/poi/hssf/record/formula/eval/LazyRefEval.java b/src/java/org/apache/poi/hssf/record/formula/eval/LazyRefEval.java index 4841725249..436bb4ccbe 100644 --- a/src/java/org/apache/poi/hssf/record/formula/eval/LazyRefEval.java +++ b/src/java/org/apache/poi/hssf/record/formula/eval/LazyRefEval.java @@ -18,14 +18,13 @@ package org.apache.poi.hssf.record.formula.eval; import org.apache.poi.hssf.record.formula.AreaI; -import org.apache.poi.hssf.record.formula.AreaI.OffsetArea; import org.apache.poi.hssf.record.formula.Ref3DPtg; import org.apache.poi.hssf.record.formula.RefPtg; +import org.apache.poi.hssf.record.formula.AreaI.OffsetArea; import org.apache.poi.hssf.usermodel.HSSFCell; import org.apache.poi.hssf.usermodel.HSSFFormulaEvaluator; import org.apache.poi.hssf.usermodel.HSSFRow; import org.apache.poi.hssf.usermodel.HSSFSheet; -import org.apache.poi.hssf.usermodel.HSSFWorkbook; import org.apache.poi.hssf.util.CellReference; /** @@ -35,18 +34,18 @@ import org.apache.poi.hssf.util.CellReference; public final class LazyRefEval extends RefEvalBase { private final HSSFSheet _sheet; - private final HSSFWorkbook _workbook; + private final HSSFFormulaEvaluator _evaluator; - public LazyRefEval(RefPtg ptg, HSSFSheet sheet, HSSFWorkbook workbook) { + public LazyRefEval(RefPtg ptg, HSSFSheet sheet, HSSFFormulaEvaluator evaluator) { super(ptg.getRow(), ptg.getColumn()); _sheet = sheet; - _workbook = workbook; + _evaluator = evaluator; } - public LazyRefEval(Ref3DPtg ptg, HSSFSheet sheet, HSSFWorkbook workbook) { + public LazyRefEval(Ref3DPtg ptg, HSSFSheet sheet, HSSFFormulaEvaluator evaluator) { super(ptg.getRow(), ptg.getColumn()); _sheet = sheet; - _workbook = workbook; + _evaluator = evaluator; } public ValueEval getInnerValueEval() { @@ -61,7 +60,7 @@ public final class LazyRefEval extends RefEvalBase { if (cell == null) { return BlankEval.INSTANCE; } - return HSSFFormulaEvaluator.getEvalForCell(cell, _sheet, _workbook); + return _evaluator.getEvalForCell(cell, _sheet); } public AreaEval offset(int relFirstRowIx, int relLastRowIx, int relFirstColIx, int relLastColIx) { @@ -69,14 +68,14 @@ public final class LazyRefEval extends RefEvalBase { AreaI area = new OffsetArea(getRow(), getColumn(), relFirstRowIx, relLastRowIx, relFirstColIx, relLastColIx); - return new LazyAreaEval(area, _sheet, _workbook); + return new LazyAreaEval(area, _sheet, _evaluator); } public String toString() { CellReference cr = new CellReference(getRow(), getColumn()); StringBuffer sb = new StringBuffer(); sb.append(getClass().getName()).append("["); - String sheetName = _workbook.getSheetName(_workbook.getSheetIndex(_sheet)); + String sheetName = _evaluator.getSheetName(_sheet); sb.append(sheetName); sb.append('!'); sb.append(cr.formatAsString()); diff --git a/src/java/org/apache/poi/hssf/usermodel/EvaluationCache.java b/src/java/org/apache/poi/hssf/usermodel/EvaluationCache.java new file mode 100644 index 0000000000..97c4d6eed3 --- /dev/null +++ b/src/java/org/apache/poi/hssf/usermodel/EvaluationCache.java @@ -0,0 +1,95 @@ +/* ==================================================================== + 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.poi.hssf.usermodel; + +import java.util.HashMap; +import java.util.Map; + +import org.apache.poi.hssf.record.formula.eval.ValueEval; + +/** + * Performance optimisation for {@link HSSFFormulaEvaluator}. This class stores previously + * calculated values of already visited cells, to avoid unnecessary re-calculation when the + * same cells are referenced multiple times + * + * + * @author Josh Micich + */ +final class EvaluationCache { + private static final class Key { + + private final int _sheetIndex; + private final int _srcRowNum; + private final int _srcColNum; + private final int _hashCode; + + public Key(int sheetIndex, int srcRowNum, int srcColNum) { + _sheetIndex = sheetIndex; + _srcRowNum = srcRowNum; + _srcColNum = srcColNum; + _hashCode = sheetIndex + srcRowNum + srcColNum; + } + + public int hashCode() { + return _hashCode; + } + + public boolean equals(Object obj) { + Key other = (Key) obj; + if (_hashCode != other._hashCode) { + return false; + } + if (_sheetIndex != other._sheetIndex) { + return false; + } + if (_srcRowNum != other._srcRowNum) { + return false; + } + if (_srcColNum != other._srcColNum) { + return false; + } + return true; + } + } + + private final Map _valuesByKey; + + /* package */EvaluationCache() { + _valuesByKey = new HashMap(); + } + + public ValueEval getValue(int sheetIndex, int srcRowNum, int srcColNum) { + Key key = new Key(sheetIndex, srcRowNum, srcColNum); + return (ValueEval) _valuesByKey.get(key); + } + + public void setValue(int sheetIndex, int srcRowNum, int srcColNum, ValueEval value) { + Key key = new Key(sheetIndex, srcRowNum, srcColNum); + if (_valuesByKey.containsKey(key)) { + throw new RuntimeException("Already have cached value for this cell"); + } + _valuesByKey.put(key, value); + } + + /** + * Should be called whenever there are changes to input cells in the evaluated workbook. + */ + public void clear() { + _valuesByKey.clear(); + } +} diff --git a/src/java/org/apache/poi/hssf/usermodel/HSSFFormulaEvaluator.java b/src/java/org/apache/poi/hssf/usermodel/HSSFFormulaEvaluator.java index 55cd04ee11..be0262292f 100644 --- a/src/java/org/apache/poi/hssf/usermodel/HSSFFormulaEvaluator.java +++ b/src/java/org/apache/poi/hssf/usermodel/HSSFFormulaEvaluator.java @@ -56,19 +56,67 @@ import org.apache.poi.hssf.record.formula.eval.OperationEval; import org.apache.poi.hssf.record.formula.eval.RefEval; import org.apache.poi.hssf.record.formula.eval.StringEval; import org.apache.poi.hssf.record.formula.eval.ValueEval; +import org.apache.poi.hssf.util.CellReference; /** + * Evaluates formula cells.

+ * + * For performance reasons, this class keeps a cache of all previously calculated intermediate + * cell values. Be sure to call {@link #clearCache()} if any workbook cells are changed between + * calls to evaluate~ methods on this class. + * * @author Amol S. Deshmukh < amolweb at ya hoo dot com > - * + * @author Josh Micich */ public class HSSFFormulaEvaluator { + /** + * used to track the number of evaluations + */ + private static final class Counter { + public int value; + public Counter() { + value = 0; + } + } + private final HSSFSheet _sheet; private final HSSFWorkbook _workbook; + private final EvaluationCache _cache; + + private Counter _evaluationCounter; public HSSFFormulaEvaluator(HSSFSheet sheet, HSSFWorkbook workbook) { + this(sheet, workbook, new EvaluationCache(), new Counter()); + } + + private HSSFFormulaEvaluator(HSSFSheet sheet, HSSFWorkbook workbook, EvaluationCache cache, Counter evaluationCounter) { _sheet = sheet; _workbook = workbook; + _cache = cache; + _evaluationCounter = evaluationCounter; + } + + /** + * for debug use. Used in toString methods + */ + public String getSheetName(HSSFSheet sheet) { + return _workbook.getSheetName(_workbook.getSheetIndex(sheet)); + } + /** + * for debug/test use + */ + /* package */ int getEvaluationCount() { + return _evaluationCounter.value; + } + + private static boolean isDebugLogEnabled() { + return false; + } + private static void logDebug(String s) { + if (isDebugLogEnabled()) { + System.out.println(s); + } } /** @@ -82,6 +130,14 @@ public class HSSFFormulaEvaluator { } } + /** + * Should be called whenever there are changes to input cells in the evaluated workbook. + * Failure to call this method after changing cell values will cause incorrect behaviour + * of the evaluate~ methods of this class + */ + public void clearCache() { + _cache.clear(); + } /** * Returns an underlying FormulaParser, for the specified @@ -118,7 +174,7 @@ public class HSSFFormulaEvaluator { retval.setErrorValue(cell.getErrorCellValue()); break; case HSSFCell.CELL_TYPE_FORMULA: - retval = getCellValueForEval(internalEvaluate(cell, _sheet, _workbook)); + retval = getCellValueForEval(internalEvaluate(cell, _sheet)); break; case HSSFCell.CELL_TYPE_NUMERIC: retval = new CellValue(HSSFCell.CELL_TYPE_NUMERIC); @@ -156,7 +212,7 @@ public class HSSFFormulaEvaluator { if (cell != null) { switch (cell.getCellType()) { case HSSFCell.CELL_TYPE_FORMULA: - CellValue cv = getCellValueForEval(internalEvaluate(cell, _sheet, _workbook)); + CellValue cv = getCellValueForEval(internalEvaluate(cell, _sheet)); switch (cv.getCellType()) { case HSSFCell.CELL_TYPE_BOOLEAN: cell.setCellValue(cv.getBooleanValue()); @@ -201,7 +257,7 @@ public class HSSFFormulaEvaluator { if (cell != null) { switch (cell.getCellType()) { case HSSFCell.CELL_TYPE_FORMULA: - CellValue cv = getCellValueForEval(internalEvaluate(cell, _sheet, _workbook)); + CellValue cv = getCellValueForEval(internalEvaluate(cell, _sheet)); switch (cv.getCellType()) { case HSSFCell.CELL_TYPE_BOOLEAN: cell.setCellType(HSSFCell.CELL_TYPE_BOOLEAN); @@ -299,28 +355,40 @@ public class HSSFFormulaEvaluator { * else a runtime exception will be thrown somewhere inside the method. * (Hence this is a private method.) */ - private static ValueEval internalEvaluate(HSSFCell srcCell, HSSFSheet sheet, HSSFWorkbook workbook) { + private ValueEval internalEvaluate(HSSFCell srcCell, HSSFSheet sheet) { int srcRowNum = srcCell.getRowIndex(); int srcColNum = srcCell.getCellNum(); ValueEval result; + int sheetIndex = _workbook.getSheetIndex(sheet); + result = _cache.getValue(sheetIndex, srcRowNum, srcColNum); + if (result != null) { + return result; + } + _evaluationCounter.value++; + EvaluationCycleDetector tracker = EvaluationCycleDetectorManager.getTracker(); - if(!tracker.startEvaluate(workbook, sheet, srcRowNum, srcColNum)) { + if(!tracker.startEvaluate(_workbook, sheet, srcRowNum, srcColNum)) { return ErrorEval.CIRCULAR_REF_ERROR; } try { - result = evaluateCell(workbook, sheet, srcRowNum, (short)srcColNum, srcCell.getCellFormula()); + result = evaluateCell(srcRowNum, (short)srcColNum, srcCell.getCellFormula()); } finally { - tracker.endEvaluate(workbook, sheet, srcRowNum, srcColNum); + tracker.endEvaluate(_workbook, sheet, srcRowNum, srcColNum); + _cache.setValue(sheetIndex, srcRowNum, srcColNum, result); + } + if (isDebugLogEnabled()) { + String sheetName = _workbook.getSheetName(sheetIndex); + CellReference cr = new CellReference(srcRowNum, srcColNum); + logDebug("Evaluated " + sheetName + "!" + cr.formatAsString() + " to " + result.toString()); } return result; } - private static ValueEval evaluateCell(HSSFWorkbook workbook, HSSFSheet sheet, - int srcRowNum, short srcColNum, String cellFormulaText) { + private ValueEval evaluateCell(int srcRowNum, short srcColNum, String cellFormulaText) { - Ptg[] ptgs = FormulaParser.parse(cellFormulaText, workbook); + Ptg[] ptgs = FormulaParser.parse(cellFormulaText, _workbook); Stack stack = new Stack(); for (int i = 0, iSize = ptgs.length; i < iSize; i++) { @@ -352,13 +420,15 @@ public class HSSFFormulaEvaluator { Eval p = (Eval) stack.pop(); ops[j] = p; } - opResult = invokeOperation(operation, ops, srcRowNum, srcColNum, workbook, sheet); + logDebug("invoke " + operation + " (nAgs=" + numops + ")"); + opResult = invokeOperation(operation, ops, srcRowNum, srcColNum, _workbook, _sheet); } else { - opResult = getEvalForPtg(ptg, sheet, workbook); + opResult = getEvalForPtg(ptg, _sheet); } if (opResult == null) { throw new RuntimeException("Evaluation result must not be null"); } + logDebug("push " + opResult); stack.push(opResult); } @@ -415,30 +485,38 @@ public class HSSFFormulaEvaluator { return operation.evaluate(ops, srcRowNum, srcColNum); } + private HSSFSheet getOtherSheet(int externSheetIndex) { + Workbook wb = _workbook.getWorkbook(); + return _workbook.getSheetAt(wb.getSheetIndexFromExternSheetIndex(externSheetIndex)); + } + private HSSFFormulaEvaluator createEvaluatorForAnotherSheet(HSSFSheet sheet) { + return new HSSFFormulaEvaluator(sheet, _workbook, _cache, _evaluationCounter); + } + /** * returns an appropriate Eval impl instance for the Ptg. The Ptg must be * one of: Area3DPtg, AreaPtg, ReferencePtg, Ref3DPtg, IntPtg, NumberPtg, * StringPtg, BoolPtg
special Note: OperationPtg subtypes cannot be * passed here! */ - private static Eval getEvalForPtg(Ptg ptg, HSSFSheet sheet, HSSFWorkbook workbook) { + private Eval getEvalForPtg(Ptg ptg, HSSFSheet sheet) { if (ptg instanceof NamePtg) { // named ranges, macro functions NamePtg namePtg = (NamePtg) ptg; - int numberOfNames = workbook.getNumberOfNames(); + int numberOfNames = _workbook.getNumberOfNames(); int nameIndex = namePtg.getIndex(); if(nameIndex < 0 || nameIndex >= numberOfNames) { - throw new RuntimeException("Bad name index (" + nameIndex + throw new RuntimeException("Bad name index (" + nameIndex + "). Allowed range is (0.." + (numberOfNames-1) + ")"); } - NameRecord nameRecord = workbook.getWorkbook().getNameRecord(nameIndex); + NameRecord nameRecord = _workbook.getWorkbook().getNameRecord(nameIndex); if (nameRecord.isFunctionName()) { return new NameEval(nameRecord.getNameText()); } if (nameRecord.hasFormula()) { - return evaluateNameFormula(nameRecord.getNameDefinition(), sheet, workbook); + return evaluateNameFormula(nameRecord.getNameDefinition(), sheet); } - + throw new RuntimeException("Don't now how to evalate name '" + nameRecord.getNameText() + "'"); } if (ptg instanceof NameXPtg) { @@ -446,22 +524,20 @@ public class HSSFFormulaEvaluator { return new NameXEval(nameXPtg.getSheetRefIndex(), nameXPtg.getNameIndex()); } if (ptg instanceof RefPtg) { - return new LazyRefEval(((RefPtg) ptg), sheet, workbook); + return new LazyRefEval(((RefPtg) ptg), sheet, this); } if (ptg instanceof Ref3DPtg) { Ref3DPtg refPtg = (Ref3DPtg) ptg; - Workbook wb = workbook.getWorkbook(); - HSSFSheet xsheet = workbook.getSheetAt(wb.getSheetIndexFromExternSheetIndex(refPtg.getExternSheetIndex())); - return new LazyRefEval(refPtg, xsheet, workbook); + HSSFSheet xsheet = getOtherSheet(refPtg.getExternSheetIndex()); + return new LazyRefEval(refPtg, xsheet, createEvaluatorForAnotherSheet(xsheet)); } if (ptg instanceof AreaPtg) { - return new LazyAreaEval(((AreaPtg) ptg), sheet, workbook); + return new LazyAreaEval(((AreaPtg) ptg), sheet, this); } if (ptg instanceof Area3DPtg) { Area3DPtg a3dp = (Area3DPtg) ptg; - Workbook wb = workbook.getWorkbook(); - HSSFSheet xsheet = workbook.getSheetAt(wb.getSheetIndexFromExternSheetIndex(a3dp.getExternSheetIndex())); - return new LazyAreaEval(a3dp, xsheet, workbook); + HSSFSheet xsheet = getOtherSheet(a3dp.getExternSheetIndex()); + return new LazyAreaEval(a3dp, xsheet, createEvaluatorForAnotherSheet(xsheet)); } if (ptg instanceof IntPtg) { @@ -479,18 +555,17 @@ public class HSSFFormulaEvaluator { if (ptg instanceof ErrPtg) { return ErrorEval.valueOf(((ErrPtg) ptg).getErrorCode()); } - if (ptg instanceof UnknownPtg) { + if (ptg instanceof UnknownPtg) { // TODO - remove UnknownPtg throw new RuntimeException("UnknownPtg not allowed"); } throw new RuntimeException("Unexpected ptg class (" + ptg.getClass().getName() + ")"); } - private static Eval evaluateNameFormula(Ptg[] ptgs, HSSFSheet sheet, - HSSFWorkbook workbook) { + private Eval evaluateNameFormula(Ptg[] ptgs, HSSFSheet sheet) { if (ptgs.length > 1) { throw new RuntimeException("Complex name formulas not supported yet"); } - return getEvalForPtg(ptgs[0], sheet, workbook); + return getEvalForPtg(ptgs[0], sheet); } /** @@ -502,7 +577,7 @@ public class HSSFFormulaEvaluator { * @param sheet * @param workbook */ - public static ValueEval getEvalForCell(HSSFCell cell, HSSFSheet sheet, HSSFWorkbook workbook) { + public ValueEval getEvalForCell(HSSFCell cell, HSSFSheet sheet) { if (cell == null) { return BlankEval.INSTANCE; @@ -513,7 +588,7 @@ public class HSSFFormulaEvaluator { case HSSFCell.CELL_TYPE_STRING: return new StringEval(cell.getRichStringCellValue().getString()); case HSSFCell.CELL_TYPE_FORMULA: - return internalEvaluate(cell, sheet, workbook); + return internalEvaluate(cell, sheet); case HSSFCell.CELL_TYPE_BOOLEAN: return BoolEval.valueOf(cell.getBooleanCellValue()); case HSSFCell.CELL_TYPE_BLANK: diff --git a/src/testcases/org/apache/poi/hssf/data/45376.xls b/src/testcases/org/apache/poi/hssf/data/45376.xls deleted file mode 100644 index 74602fd0b7..0000000000 Binary files a/src/testcases/org/apache/poi/hssf/data/45376.xls and /dev/null differ diff --git a/src/testcases/org/apache/poi/hssf/record/formula/functions/TestDate.java b/src/testcases/org/apache/poi/hssf/record/formula/functions/TestDate.java index 83c9fcd34c..68bc43154a 100644 --- a/src/testcases/org/apache/poi/hssf/record/formula/functions/TestDate.java +++ b/src/testcases/org/apache/poi/hssf/record/formula/functions/TestDate.java @@ -28,29 +28,29 @@ import org.apache.poi.hssf.usermodel.HSSFWorkbook; * @author Pavel Krupets (pkrupets at palmtreebusiness dot com) */ public final class TestDate extends TestCase { - + private HSSFCell cell11; private HSSFFormulaEvaluator evaluator; public void setUp() { HSSFWorkbook wb = new HSSFWorkbook(); HSSFSheet sheet = wb.createSheet("new sheet"); - cell11 = sheet.createRow(0).createCell(0); + cell11 = sheet.createRow(0).createCell(0); cell11.setCellType(HSSFCell.CELL_TYPE_FORMULA); evaluator = new HSSFFormulaEvaluator(sheet, wb); } - - /** - * Test disabled pending a fix in the formula evaluator - * TODO - create MissingArgEval and modify the formula evaluator to handle this - */ + + /** + * Test disabled pending a fix in the formula evaluator + * TODO - create MissingArgEval and modify the formula evaluator to handle this + */ public void DISABLEDtestSomeArgumentsMissing() { confirm("DATE(, 1, 0)", 0.0); confirm("DATE(, 1, 1)", 1.0); } - + public void testValid() { - + confirm("DATE(1900, 1, 1)", 1); confirm("DATE(1900, 1, 32)", 32); confirm("DATE(1900, 222, 1)", 6727); @@ -58,7 +58,7 @@ public final class TestDate extends TestCase { confirm("DATE(2000, 1, 222)", 36747.00); confirm("DATE(2007, 1, 1)", 39083); } - + public void testBugDate() { confirm("DATE(1900, 2, 29)", 60); confirm("DATE(1900, 2, 30)", 61); @@ -66,7 +66,7 @@ public final class TestDate extends TestCase { confirm("DATE(1900, 1, 2222)", 2222); confirm("DATE(1900, 1, 22222)", 22222); } - + public void testPartYears() { confirm("DATE(4, 1, 1)", 1462.00); confirm("DATE(14, 1, 1)", 5115.00); @@ -74,10 +74,11 @@ public final class TestDate extends TestCase { confirm("DATE(1004, 1, 1)", 366705.00); } - private void confirm(String formulaText, double expectedResult) { + private void confirm(String formulaText, double expectedResult) { cell11.setCellFormula(formulaText); + evaluator.clearCache(); double actualValue = evaluator.evaluate(cell11).getNumberValue(); - assertEquals(expectedResult, actualValue, 0); - } + assertEquals(expectedResult, actualValue, 0); + } } diff --git a/src/testcases/org/apache/poi/hssf/usermodel/TestFormulaEvaluatorBugs.java b/src/testcases/org/apache/poi/hssf/usermodel/TestFormulaEvaluatorBugs.java index 6ebcf96bb6..f1d838efa3 100644 --- a/src/testcases/org/apache/poi/hssf/usermodel/TestFormulaEvaluatorBugs.java +++ b/src/testcases/org/apache/poi/hssf/usermodel/TestFormulaEvaluatorBugs.java @@ -19,6 +19,8 @@ package org.apache.poi.hssf.usermodel; import java.io.File; import java.io.FileOutputStream; +import java.util.Calendar; +import java.util.GregorianCalendar; import java.util.Iterator; import junit.framework.AssertionFailedError; @@ -35,6 +37,7 @@ import org.apache.poi.hssf.record.formula.Ptg; */ public final class TestFormulaEvaluatorBugs extends TestCase { + private static final boolean OUTPUT_TEST_FILES = false; private String tmpDirName; protected void setUp() { @@ -65,13 +68,15 @@ public final class TestFormulaEvaluatorBugs extends TestCase { HSSFFormulaEvaluator.evaluateAllFormulaCells(wb); assertEquals(4.2 * 25, row.getCell(3).getNumericCellValue(), 0.0001); - // Save - File existing = new File(tmpDirName, "44636-existing.xls"); - FileOutputStream out = new FileOutputStream(existing); - wb.write(out); - out.close(); - System.err.println("Existing file for bug #44636 written to " + existing.toString()); - + FileOutputStream out; + if (OUTPUT_TEST_FILES) { + // Save + File existing = new File(tmpDirName, "44636-existing.xls"); + out = new FileOutputStream(existing); + wb.write(out); + out.close(); + System.err.println("Existing file for bug #44636 written to " + existing.toString()); + } // Now, do a new file from scratch wb = new HSSFWorkbook(); sheet = wb.createSheet(); @@ -86,12 +91,14 @@ public final class TestFormulaEvaluatorBugs extends TestCase { HSSFFormulaEvaluator.evaluateAllFormulaCells(wb); assertEquals(5.4, row.getCell(0).getNumericCellValue(), 0.0001); - // Save - File scratch = new File(tmpDirName, "44636-scratch.xls"); - out = new FileOutputStream(scratch); - wb.write(out); - out.close(); - System.err.println("New file for bug #44636 written to " + scratch.toString()); + if (OUTPUT_TEST_FILES) { + // Save + File scratch = new File(tmpDirName, "44636-scratch.xls"); + out = new FileOutputStream(scratch); + wb.write(out); + out.close(); + System.err.println("New file for bug #44636 written to " + scratch.toString()); + } } /** @@ -281,64 +288,39 @@ public final class TestFormulaEvaluatorBugs extends TestCase { } /** - * Apparently, each subsequent call takes longer, which is very - * odd. - * We think it's because the formulas are recursive and crazy + * The HSSFFormula evaluator performance benefits greatly from caching of intermediate cell values */ - public void DISABLEDtestSlowEvaluate45376() throws Exception { - HSSFWorkbook wb = HSSFTestDataSamples.openSampleWorkbook("45376.xls"); + public void testSlowEvaluate45376() { - final String SHEET_NAME = "Eingabe"; - final int row = 6; - final HSSFSheet sheet = wb.getSheet(SHEET_NAME); - - int firstCol = 4; - int lastCol = 14; - long[] timings = new long[lastCol-firstCol+1]; - long[] rtimings = new long[lastCol-firstCol+1]; - - long then, now; - - final HSSFRow excelRow = sheet.getRow(row); - for(int i = firstCol; i <= lastCol; i++) { - final HSSFCell excelCell = excelRow.getCell(i); - final HSSFFormulaEvaluator evaluator = new - HSSFFormulaEvaluator(sheet, wb); - - now = System.currentTimeMillis(); - evaluator.evaluate(excelCell); - then = System.currentTimeMillis(); - timings[i-firstCol] = (then-now); - System.err.println("Col " + i + " took " + (then-now) + "ms"); - } - for(int i = lastCol; i >= firstCol; i--) { - final HSSFCell excelCell = excelRow.getCell(i); - final HSSFFormulaEvaluator evaluator = new - HSSFFormulaEvaluator(sheet, wb); - - now = System.currentTimeMillis(); - evaluator.evaluate(excelCell); - then = System.currentTimeMillis(); - rtimings[i-firstCol] = (then-now); - System.err.println("Col " + i + " took " + (then-now) + "ms"); - } - - // The timings for each should be about the same - long avg = 0; - for(int i=0; i 1.5*avg) { - System.err.println("Doing col " + (i+firstCol) + - " took " + timings[i] + "ms, vs avg " + - avg + "ms" - ); - } - } + // Firstly set up a sequence of formula cells where each depends on the previous multiple + // times. Without caching, each subsequent cell take about 4 times longer to evaluate. + HSSFWorkbook wb = new HSSFWorkbook(); + HSSFSheet sheet = wb.createSheet("Sheet1"); + HSSFRow row = sheet.createRow(0); + for(int i=1; i<10; i++) { + HSSFCell cell = row.createCell(i); + char prevCol = (char) ('A' + i-1); + String prevCell = prevCol + "1"; + // this formula is inspired by the offending formula of the attachment for bug 45376 + String formula = "IF(DATE(YEAR(" + prevCell + "),MONTH(" + prevCell + ")+1,1)<=$D$3," + + "DATE(YEAR(" + prevCell + "),MONTH(" + prevCell + ")+1,1),NA())"; + cell.setCellFormula(formula); + + } + Calendar cal = new GregorianCalendar(2000, 0, 1, 0, 0, 0); + row.createCell(0).setCellValue(cal); + + // Choose cell A9, so that the failing test case doesn't take too long to execute. + HSSFCell cell = row.getCell(8); + HSSFFormulaEvaluator evaluator = new HSSFFormulaEvaluator(sheet, wb); + evaluator.evaluate(cell); + int evalCount = evaluator.getEvaluationCount(); + // With caching, the evaluationCount is 8 which is a big improvement + if (evalCount > 10) { + // Without caching, evaluating cell 'A9' takes 21845 evaluations which consumes + // much time (~3 sec on Core 2 Duo 2.2GHz) + System.err.println("Cell A9 took " + evalCount + " intermediate evaluations"); + throw new AssertionFailedError("Identifed bug 45376 - Formula evaluator should cache values"); + } } } \ No newline at end of file