]> source.dussan.org Git - poi.git/commitdiff
Fix for bug 45376 - added caching to HSSFFormulaEvaluator
authorJosh Micich <josh@apache.org>
Thu, 4 Sep 2008 23:16:15 +0000 (23:16 +0000)
committerJosh Micich <josh@apache.org>
Thu, 4 Sep 2008 23:16:15 +0000 (23:16 +0000)
git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@692300 13f79535-47bb-0310-9956-ffa450edef68

src/java/org/apache/poi/hssf/record/formula/eval/LazyAreaEval.java
src/java/org/apache/poi/hssf/record/formula/eval/LazyRefEval.java
src/java/org/apache/poi/hssf/usermodel/EvaluationCache.java [new file with mode: 0644]
src/java/org/apache/poi/hssf/usermodel/HSSFFormulaEvaluator.java
src/testcases/org/apache/poi/hssf/data/45376.xls [deleted file]
src/testcases/org/apache/poi/hssf/record/formula/functions/TestDate.java
src/testcases/org/apache/poi/hssf/usermodel/TestFormulaEvaluatorBugs.java

index e43a97e4058bf0b17f590638ba848a70e0cfd926..e2dc42e056ba71e5c5517554e12436940f94dbbc 100644 (file)
@@ -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());
index 48417252493a46fbfa0b09f03e6fd6a6bc4bedf0..436bb4ccbe9966807f6155fe9c99f8c5b70135e8 100644 (file)
 package org.apache.poi.hssf.record.formula.eval;\r
 \r
 import org.apache.poi.hssf.record.formula.AreaI;\r
-import org.apache.poi.hssf.record.formula.AreaI.OffsetArea;\r
 import org.apache.poi.hssf.record.formula.Ref3DPtg;\r
 import org.apache.poi.hssf.record.formula.RefPtg;\r
+import org.apache.poi.hssf.record.formula.AreaI.OffsetArea;\r
 import org.apache.poi.hssf.usermodel.HSSFCell;\r
 import org.apache.poi.hssf.usermodel.HSSFFormulaEvaluator;\r
 import org.apache.poi.hssf.usermodel.HSSFRow;\r
 import org.apache.poi.hssf.usermodel.HSSFSheet;\r
-import org.apache.poi.hssf.usermodel.HSSFWorkbook;\r
 import org.apache.poi.hssf.util.CellReference;\r
 \r
 /**\r
@@ -35,18 +34,18 @@ import org.apache.poi.hssf.util.CellReference;
 public final class LazyRefEval extends RefEvalBase {\r
 \r
        private final HSSFSheet _sheet;\r
-       private final HSSFWorkbook _workbook;\r
+       private final HSSFFormulaEvaluator _evaluator;\r
 \r
 \r
-       public LazyRefEval(RefPtg ptg, HSSFSheet sheet, HSSFWorkbook workbook) {\r
+       public LazyRefEval(RefPtg ptg, HSSFSheet sheet, HSSFFormulaEvaluator evaluator) {\r
                super(ptg.getRow(), ptg.getColumn());\r
                _sheet = sheet;\r
-               _workbook = workbook;\r
+               _evaluator = evaluator;\r
        }\r
-       public LazyRefEval(Ref3DPtg ptg, HSSFSheet sheet, HSSFWorkbook workbook) {\r
+       public LazyRefEval(Ref3DPtg ptg, HSSFSheet sheet, HSSFFormulaEvaluator evaluator) {\r
                super(ptg.getRow(), ptg.getColumn());\r
                _sheet = sheet;\r
-               _workbook = workbook;\r
+               _evaluator = evaluator;\r
        }\r
 \r
        public ValueEval getInnerValueEval() {\r
@@ -61,7 +60,7 @@ public final class LazyRefEval extends RefEvalBase {
                if (cell == null) {\r
                        return BlankEval.INSTANCE;\r
                }\r
-               return HSSFFormulaEvaluator.getEvalForCell(cell, _sheet, _workbook);\r
+               return _evaluator.getEvalForCell(cell, _sheet);\r
        }\r
        \r
        public AreaEval offset(int relFirstRowIx, int relLastRowIx, int relFirstColIx, int relLastColIx) {\r
@@ -69,14 +68,14 @@ public final class LazyRefEval extends RefEvalBase {
                AreaI area = new OffsetArea(getRow(), getColumn(),\r
                                relFirstRowIx, relLastRowIx, relFirstColIx, relLastColIx);\r
 \r
-               return new LazyAreaEval(area, _sheet, _workbook);\r
+               return new LazyAreaEval(area, _sheet, _evaluator);\r
        }\r
        \r
        public String toString() {\r
                CellReference cr = new CellReference(getRow(), getColumn());\r
                StringBuffer sb = new StringBuffer();\r
                sb.append(getClass().getName()).append("[");\r
-               String sheetName = _workbook.getSheetName(_workbook.getSheetIndex(_sheet));\r
+               String sheetName = _evaluator.getSheetName(_sheet);\r
                sb.append(sheetName);\r
                sb.append('!');\r
                sb.append(cr.formatAsString());\r
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 (file)
index 0000000..97c4d6e
--- /dev/null
@@ -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();
+       }
+}
index 55cd04ee114d8491d63ecd9495bc192a95a62a11..be0262292fabedd663471d9211a58c133bb32276 100644 (file)
@@ -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.<p/>
+ * 
+ * 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 &lt; amolweb at ya hoo dot com &gt;
- *
+ * @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 <br/>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 (file)
index 74602fd..0000000
Binary files a/src/testcases/org/apache/poi/hssf/data/45376.xls and /dev/null differ
index 83c9fcd34c17cba5ac0968084f2359d875860fd1..68bc43154ac17a1c1266b8413ab1cf212e0fadf6 100644 (file)
@@ -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);
+    }
 }
 
index 6ebcf96bb66b194fcc5bab96b16a0f8cd13921bf..f1d838efa398c6ac664c5c37e4716792271694d5 100644 (file)
@@ -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<timings.length; i++) {
-               avg += timings[i];
-        }
-        avg = (long)( ((double)avg) / timings.length );
-        
-        // Warn if any took more then 1.5 the average
-        // TODO - replace with assert or similar
-        for(int i=0; i<timings.length; i++) {
-               if(timings[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