From bae56b7be7a1c03c44a3f7f2918e156172e9bccf Mon Sep 17 00:00:00 2001 From: Josh Micich Date: Thu, 4 Sep 2008 23:16:15 +0000 Subject: [PATCH] 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 --- .../record/formula/eval/LazyAreaEval.java | 13 +- .../hssf/record/formula/eval/LazyRefEval.java | 19 ++- .../poi/hssf/usermodel/EvaluationCache.java | 95 ++++++++++++ .../hssf/usermodel/HSSFFormulaEvaluator.java | 141 ++++++++++++++---- .../org/apache/poi/hssf/data/45376.xls | Bin 27648 -> 0 bytes .../record/formula/functions/TestDate.java | 29 ++-- .../usermodel/TestFormulaEvaluatorBugs.java | 122 +++++++-------- 7 files changed, 285 insertions(+), 134 deletions(-) create mode 100644 src/java/org/apache/poi/hssf/usermodel/EvaluationCache.java delete mode 100644 src/testcases/org/apache/poi/hssf/data/45376.xls 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 74602fd0b7caf9e946da734e108494e566f914d8..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 27648 zcmeHQdvp}neZI3=2}vMW34y_YSXR72uqz260g`s5U4d+b2*`%Svc*bTjEscj)nbF6 ziyw`fIIUwliJLevXOyYkateDdO7i@dEx6o|7EC88(?w{T98 zTR?~vIH7)aVq!vpLg4g+?T@Gi-a%QHqwxwLen=sNZ9W513@L$>LS{l{LCPSrA#)%w z0Z|UAfXszdLgqo{LoR|WfGmVu3|RzO3|RuHf-HpuALu%^|J1qiE5ClpS_v>q=t`Ox3~oL&@OtQw~NbwoNo&W%iXU0%0y z{kdUO?zzZ?ofl&ScCMZM*0eeKJICBGCB^8)MTO!!OMf!s#Zx%NK=5nf6HoYG#9oLq zkX|vye&3bMWDX4_$Ag_$r-p;S4WA!)G;r%=JoIJ3laRwYNA_)uy*6-)Q0hBD(L z$+5B2g9tqvV+}*YBCww;_m9i?&&z!?nA6sfShVZ=kw%2zy=%WOD#Lw3?n81v8+%K9 zTgLgNL?Rf$UPAbZTkm003-R$nD+a3_5=k+PQRsKe`o(g=eDTkauOFlGx!98^>a@!2 zo1AhKRy}d+^Q=k(`w~iXZZ;?1bHptXQ>9!w1l_Wo8mH%4YFB^o_hbvx-8qkUA`XHz3WabSQHHyu4EfLGfjJRn_aOtHZs)P~x&Zp{>0&xoVYvh4>0;(;McBJ*mMf z^8>CFUlq&jfE&X3>1m|OzfOEj;07-9(fHQfR5e`XkBNuHHX-_I)&_Hn)>nhFPJ$RJ zR1>nV24VVYqJ1@0{#b2!DQj8#5d_JJGB(=%56?)RsL%6D1GPL5V_Tk63Sr9 z#bb1@b6O}TEp>4++#2yXZY}xYa#PF++#;S3O*vPqGRv(t7jdhf#nFw}BA(1kp)cOo zk5m}~k@eJoh z$FcR+jEwZw3=UTL%fz=hc^bji8$5pew450U@gEcDi|%$RNZf)h@i!b|YMeJ@Q}}b? zOw|TrxMIPqCB9fe!xz?V&d2krD#I$X!Fc+1<56K;0;s{HGaj?WjB`R10N zxKo~d+*JN*hF6=X{GloEpb+@ylt+~l;f>`)>mTz4qm1FHXX1e%&>WoY1^lLR);`i- zYqyJoFZ1AKm}jZ*m>=eZSM|f|s;E%*woaxq8{?qGroAZ;Q28o6Cw%!i!k3F6O5~pu zewjBs+e_vfKxj8S*P;~e%~zL4E>j`AvVY-}Gh<(uZT~uad0zU?b(QCppZTIr2AlE=B(VtK607RsZ@3VEED^~>Y@?2L&6mJf;D zi8Z$sC^XmR5<^F>PWGqn#*$qGe~(#yWNP^@LTM~J-xsZ7@gkJa3zm3WHYN)!7x%WO z(#gTp1gxQ0{1yv&>{A29^B{Iy9Jr3h?PZvZP>Tcs&mzH0-)b3vyT5{LM2vqy|HyPIGl*@ep1$g`GzkRu@e|Kx246|0^>*W46iDY#R zixsFqOlidlFfaO9!z||QGV~pC-z6FlqzhB?0Zh`kT;AG`1)jYk2>)*s+)*)t zi5(Zbf>?s$R6YYejnufII*WcK_7@c3hGAh@=d&~GiwAKw2MY$g67GeF$wfyQ;eFV# zZMgBs)OqB>4i26z-nr|tkO){i3ucKQz5Jg}kv(A?MK4G>sXNRE$Nw)iklXSBD0{nl zF5$IfB(dI0Kj}O&b~MvAbVPJ?@9Emxec7J&gxHhV8{fH8Ty|tAof_v^=aEb*J)BBj zhw(~ucXWu(k>u5>p5tRFamnt)_O8^R=uQu%j%1RVq0u8^clVy2J+aOn(LOpnoXn(T zls&2O(cz<#?;hhJB6cUoQt7=zH>AYQL{Cp*55te9yG937qPD(&Y;3x1%YmX zdif#-?GlVn7)SBvLm%Jo``D6gZ@+WPtF6w_!jiBL!{*I$$P}Wp;3DWgsWaBI>p8k9 zoyU6vS}(vOmS=NxKNc{aovC@QHc&4>JmKSQO^vHUL~&N^lOsRQJ2^Ors5J*G?E&OT zexnDYl8+S_Fpi~BFKD!dp+pnQyXlza@1 z-Xx5zlOK-mP(H@G%E$P4Dj#DF=3{WECt+}cCt-ZNl^>V)pL~q>qkN2y!}2jc9?Qq* zk>_Jr5SWDVaaw*jK3>bm=*j0}eEgP=VM!qulXivm17ParbJA)NFVJ;_wJnf~?6>%+ z6=2$N`XgtemX8+V$6^#ivF|LxETX{H=3#NP3XcRCihW|CU~Hd?{k=2M7tohz^Nh0? zsW1&%=28(vT1CM9V3z?AGB+XaT2}c^A@1X%;M5Bu3%USZw;D?szwf^lSO{xHfoIF7 zrmOJ0at&rO1@Zz5Q7(8x;`P=dTuLGSsL3tLr>sSIwjM{IQ&pBI;8g;0gA_-Lq0g8o z6c*v<{d1w>Y_+H^5j)`*u-Ntb%*QkPyV>6u=7 zrb`DUJm^;` zr7K)|gQVwr>A5Z)mUN|;u5{^$r003*c`jWm>G@uIzDw6h`XVoVkxSQ0dV!Z-;L@8U zz0gZ9bm<03U+kqXcInNMUgV`0xpbqX7klZ&F5M*QC0=@oOK*{Mm6xt^>8+Ap>ZO;u z^fpNcy>!r}nOD}Wj7DE&K}xkG0ay~0bcaA*;eu~&NO zl@6U%^eQjC%ArM@jJ?`RuXgCHqStuoH4ZJ>W$d+zUgM4_Yg)woNn^@dCstO)TIa-4 zW6C;*7IB&AdM~}+p|gsv@zOO8EfO;J1~0wAp|gtK=%qJ0wCIqr!(KY<&{;)CymZ8& z#daCH)=SqqbXL)IUb@bq#SR&}-b>dzbXL)uy!0lA7M(J7gO_e_=&YhQd+E&%EiRF< z8@+U+LuVD;=rNG zf=79B%PP86)4XrC;)$Tzc17h|L+ZZSBJY5__0sp)fY*UG%3CiDv2vdKkHhtr^aZALD|zWk@V0;`CKY7y7EE2(z%bRK!Zf!r}ta zR-n9=B4ngWakBpmF1$$H&j~;8--kW^a6j)-_jBYd@9Qyh&WbaIG3GqO&N+Bcpy}B*eLAaX=@V#rj!mDsL($SF&~(72vv-||u15V!pFq>)Hhti3MN6MR z(-k%yy+_f~C(!g|Y&!cnMN6MR)AMcmz(b0bK7pn$ zvgzQL6fJ!MO)s!%@ij$DpFqhik3csrZ2YXlaDD{`UIL@WYgKF6fJ!MO)s|T z=(CEJK7po}*mUqYMN6MR(^a5-d^+%*i3zrt^a(V*)S;hHwDbuy9dziEik3csrk6SN z1B#YDfu=(aeV3x8PoQbOVIqr`RkZX8G`-xRk11OE1e#vq(1#Q)eF9Cdbm*|6rB9&g zRSx~IqNPuu>D3OsT+z}e(DWLIwyQ<@1Vxy;IO7t1g5SxwwARTvpb94a3g)ba?R5@) zx1yzAq3QJw?N_w)D>Pl>&~}4Jze3Y|-9on4n<}>SD>S{)p)-n>eubv_MvsjBx}v3D zq3MW2+s!Hc3QgBK^lK`%^eZ%7=g_^1mVSk%>mB+Z6fOMc7O9z{#PLemWn{gR@k zU!m#E4jost^eZ&o=+MtATKW~5ZgS{GMN7Xz(_0+c?w``H(DYV^Uaw+Hze3a79NM;W z=~rmF*`b3fw)87B-Gb*Lvc(=Gy;MlQLes7Ctzj%FUa@h0S!aSuQrq#kjhq!j-ugSGN>4 z+r_xLrLZ|J#?>u_1ze1)TM8?8F|KYYtir{(x}~tWF2>a@g;ly3SGN>4&&9a9rLg%f z#?>u_UF2e1-BQ>B7vt)d!WOz1SGN>)v5RqaOJR##jH_D;TkK+7-BQ>R7vt*IX4zU* z6610VSGO7=++J^8%@roPl?W=`%6G5?wG@j_c!GtE3sOsAN?Y)$$)J!U|S8?HUrjdz*-DgYaYfP zN&Dj?lS0`)X?a?s&1sF+rZw7`)@Wf`qit!8R;4xClh$ZSTB8kVjn<$`2KgXfjn83gLKaN9Dq?+H)V+O3vfVCU2xB*KTunq&Z-GJ>dV4Vi+5(9Rr z0o!T7x(wJZ1J-T8b{nt{8L&MDY_9?9F<_S&u*(hDJ_EMjfPL72eZ+uWVZeF~*p&wC zDg)M+hjARD{b?JgqXlS-?W#4lsMgq)T4O6J z?fo2wvPd<596DjZZZlxF8?ZYJ*qsLKPYl>c4cJ`<>`x8Y#|+rV4cOfV>=Op;&kWd~ z8?bu}*e4Cxy$0+)19raw`;-Cuv;q5!0eirJ{e=PhtO5I+0ejGZecph5!GJxKhjARD z{b?JgqXlSEhrVdQ{?dSb$$)*?fPKY)ebs<{&44{@z`kz49x-6wFks&_V2>KG#|+rx z2J8s~_M`!O%78s>z@9N+-!fp&8nAC0u4vC$( zYixb3vE8-C7S|fvT5D`&t+9Qz#+KC@+f-|8O|7vVwZ<0I8rx25Y&EU1y|l)b(i+=H zYiu2@v0b#r7SS5pLThXVt+D=<_H(RYe;V%r7r(1jmlj`zc%IKiFyqx5Te1W#8iV-p z*klIw{KgAjWA@2Vd>fQ!gygSS_!6mx@IZU_g`q1P8p^XpO4D^-p>DA-+!4-It+6$@~6Gh z?x%bGL4<9#f15Li`kwMPJP140R*c@{T>EtMPawgis(+%Nsh=CMI(QAd7QRx&I^`oz z7L#3#k1+Yjl8-3)NRp2r`K={BV&o&mck}%x#$SfGAxJf3Ib;Q7C1e$3HDnD0?|;a5 z$kyXn1K9xC2;u8hw3u25zXMed*#v2TaKzvvK0d?S0@(`TGdn&SYk{;vqL3J*4bl#Y zLs+H`9JfPuKsq6pKrV&gEiC-BOoi#Tj~~Ku4`eT-2XYzYatPDekK>0SAF=gb9Iu32 z1?jV~q z?)l_Yh%kf@ud2hl+n@ar3(v&77N?T8o_#{ZGMV&H|55x*GxKnKN@TfhVUsW z;e3lqsE<*F%0i{3C)<+azLQ}zRXAK5Zj3bjrbYz91;y>6(pO1*^wL`Pt6(>?deo9lk6Y1<}KZwP7S68hAWbysB=&}+mk+eqO2`( zY;1HqRk>L7q(;X0Q@I22RC?$-U&Xy`M^BAsMn{Hj@W%@{FrH_-fc}5>3~T6v{!g2D zAGY_a|8o@3Jv{g9gZ_UW^)4_NmG?M~mGj;c5xd~-tT{!i?4;NgGawRbf<7WzbjUP1V#)l0K$`Ug~ zH?9RguSOaPl!>*Mz$Zhf?L-bdPap@=555!lM9F}hfdyquwo4G{^^0qO4#-a=>_k5O zvPAjc2YudX9ekrTcO-KBjvACmoom+NcvL<~H9Q+-9lBQbNh^q>T>H@Wy=a@`5Pq~_ z1T9c2kqmymJu2ssjGw_-Kd!kOHIR^Pd=%KIoY^`poZG(Z-S(}8<#fQaipvtU<4QKc zYC2#gHK?I5qzz|n;1gifmBhfRE2y{eT6kCLO6qXbSF#)RnBTtJ<#llGO{-(Or%>WJ z&Ul5Kp&!8&np$tKID}AXgcwAK$yU&X+UiDaIDNsCMy@5j8ynzyjt^snA5(@RpM@1B z%o%YE_|$EZwM;K9^4foP7E34V!fv0+F=HHl>(t6^H2eImQXgiK=U>a4VVkXAQ7H&X*p{{=8w%m&R0lNyC}qE6Q8DANiZxLxoRYdMCru{@*-%Vt-Lx zIC|Sd|N7lS|9t-!9Gpka$FdNt)8M^abUQ4k6#5g#22#V>bpcV}zq~E>UuOcq_6;40 zG`x6=eOgyL<5tpL$$_Vz3J5>LH9T64Z}j+gr$?_z9nM5vjs--Ke>^idI-I)iKtPl- zj3fs#FMOX|_rZfh1F3mO1ESnN=_|dDp9qK&6m6ow5`Bs5lEX(aaar&S=HRUA?)eyk z`1g&b504*8r7|y`B}uh^boB7y1p&OMjTssjTIR98*pIL$?wtv^Ku0la#AjF*<2x(|gmrrYO&K2=xFdnP zLsks^ObpzXIGHW~bk;hN5Vi4+#(20Mx5iCv9kH57I2&oJX^ST|WfMP(#j@@7*}~ep z64?jD)#>EONU9h2w~^G)k?|yF&2h9kX3a->yM_kRqxeG8!Ax-Pp(O5MgT44hOedU{ zNTe6vl-Ur*H)N_m8n}H|FcOVy2nW0SPy8U5-8s~sPNt9J+b_bJbK5IEG55BDw*sXL z@olYA!*m=TWeATBhQ#oA){61&+keU$5`E)Q=?aWRMXEhAisHMhS~ZdGFu@&_dgMX`0tvdwiYvRwpX1!DP$Rl_4~cSf=+epp=l za`8V_oh>@^!>SkgQ-qJ*wowwn_|v9Ce&+Il z5Z6n&dGlA;h3qjRSX?+PhOi9NiMtzt9<$wuLX?Cb1LN}!`nH|$v<{-9r6pqW6TLaC zM*hr~>*LeSe;>l0wv*xSM!X*+yRh;!zKi<50(sA}ufH?i9J;ZisXfxRsiUqY-jGPt z)HlT1YGScSxTdMTv7B4t*7?UkueNP{llrc+U8JQQ(bLsEpkDC z_O9*Cq4vi1SVz30EgZ=emUMSpbBJLho5LNO!;!EOFx%UkLz~+-C+eFLO&v}3wH+}^ zTO3qb=@mOhF*+uWWl~25Q|Vymk%OaGmX*5K7HPh*uCcB*-cTQ|Y42zX*F+-i9W{-K z`nsBMI2LYiibTTUO|hG|Uf!12>c$Gol;u4Q^ZgVOoK$_y;q^iHUwc`pQ*rsYZ8SND zv#|I=`^jn!EZ1UB-}%tLztw#x@QqL6D=@2`{so_xJOSYXJDWcW;hl>f1N+YbNCkvT z?R?gA52Ot8AY?A&VF>TKuOSkDRuzDF{!;+x=f&li&mmuWB(erEBDB_Dl zKVHHABZ7T;I@?s&?|bDZuROS6QQ*GM;QH6U^%$@JR}ena;`Q%_@HrNr)9@#2yl(d6 zEQHr~7lhY+4}?E+djP`vI0<3>JOjzCj|I?2Qkg@egTb+6`fzYBF7x_iIu(pjq58~?Wjj(g5Fo%jb3XlyzGry0h{o5yKJd0&J5G3RgLNo4^Z@bVh)CeUb^oa8pRt;PhG2$`(XdSu?GGJ D(>WJg 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 -- 2.39.5