poi-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From j...@apache.org
Subject svn commit: r692300 - in /poi/trunk/src: java/org/apache/poi/hssf/record/formula/eval/ java/org/apache/poi/hssf/usermodel/ testcases/org/apache/poi/hssf/data/ testcases/org/apache/poi/hssf/record/formula/functions/ testcases/org/apache/poi/hssf/usermodel/
Date Thu, 04 Sep 2008 23:16:16 GMT
Author: josh
Date: Thu Sep  4 16:16:15 2008
New Revision: 692300

URL: http://svn.apache.org/viewvc?rev=692300&view=rev
Log:
Fix for bug 45376 - added caching to HSSFFormulaEvaluator

Added:
    poi/trunk/src/java/org/apache/poi/hssf/usermodel/EvaluationCache.java
Removed:
    poi/trunk/src/testcases/org/apache/poi/hssf/data/45376.xls
Modified:
    poi/trunk/src/java/org/apache/poi/hssf/record/formula/eval/LazyAreaEval.java
    poi/trunk/src/java/org/apache/poi/hssf/record/formula/eval/LazyRefEval.java
    poi/trunk/src/java/org/apache/poi/hssf/usermodel/HSSFFormulaEvaluator.java
    poi/trunk/src/testcases/org/apache/poi/hssf/record/formula/functions/TestDate.java
    poi/trunk/src/testcases/org/apache/poi/hssf/usermodel/TestFormulaEvaluatorBugs.java

Modified: poi/trunk/src/java/org/apache/poi/hssf/record/formula/eval/LazyAreaEval.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/hssf/record/formula/eval/LazyAreaEval.java?rev=692300&r1=692299&r2=692300&view=diff
==============================================================================
--- poi/trunk/src/java/org/apache/poi/hssf/record/formula/eval/LazyAreaEval.java (original)
+++ poi/trunk/src/java/org/apache/poi/hssf/record/formula/eval/LazyAreaEval.java Thu Sep 
4 16:16:15 2008
@@ -23,7 +23,6 @@
 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 @@
 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 @@
 		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());

Modified: poi/trunk/src/java/org/apache/poi/hssf/record/formula/eval/LazyRefEval.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/hssf/record/formula/eval/LazyRefEval.java?rev=692300&r1=692299&r2=692300&view=diff
==============================================================================
--- poi/trunk/src/java/org/apache/poi/hssf/record/formula/eval/LazyRefEval.java (original)
+++ poi/trunk/src/java/org/apache/poi/hssf/record/formula/eval/LazyRefEval.java Thu Sep  4
16:16:15 2008
@@ -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 @@
 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 @@
 		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 @@
 		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());

Added: poi/trunk/src/java/org/apache/poi/hssf/usermodel/EvaluationCache.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/hssf/usermodel/EvaluationCache.java?rev=692300&view=auto
==============================================================================
--- poi/trunk/src/java/org/apache/poi/hssf/usermodel/EvaluationCache.java (added)
+++ poi/trunk/src/java/org/apache/poi/hssf/usermodel/EvaluationCache.java Thu Sep  4 16:16:15
2008
@@ -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();
+	}
+}

Modified: poi/trunk/src/java/org/apache/poi/hssf/usermodel/HSSFFormulaEvaluator.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/hssf/usermodel/HSSFFormulaEvaluator.java?rev=692300&r1=692299&r2=692300&view=diff
==============================================================================
--- poi/trunk/src/java/org/apache/poi/hssf/usermodel/HSSFFormulaEvaluator.java (original)
+++ poi/trunk/src/java/org/apache/poi/hssf/usermodel/HSSFFormulaEvaluator.java Thu Sep  4
16:16:15 2008
@@ -56,19 +56,67 @@
 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 @@
         }
     }
 
+    /**
+     * 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 @@
                 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 @@
         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 @@
         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 @@
      * 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 @@
                     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 @@
         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 @@
             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 @@
         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 @@
      * @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 @@
             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:

Modified: poi/trunk/src/testcases/org/apache/poi/hssf/record/formula/functions/TestDate.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/testcases/org/apache/poi/hssf/record/formula/functions/TestDate.java?rev=692300&r1=692299&r2=692300&view=diff
==============================================================================
--- poi/trunk/src/testcases/org/apache/poi/hssf/record/formula/functions/TestDate.java (original)
+++ poi/trunk/src/testcases/org/apache/poi/hssf/record/formula/functions/TestDate.java Thu
Sep  4 16:16:15 2008
@@ -28,29 +28,29 @@
  * @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 @@
         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 @@
         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 @@
         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);
+    }
 }
 

Modified: poi/trunk/src/testcases/org/apache/poi/hssf/usermodel/TestFormulaEvaluatorBugs.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/testcases/org/apache/poi/hssf/usermodel/TestFormulaEvaluatorBugs.java?rev=692300&r1=692299&r2=692300&view=diff
==============================================================================
--- poi/trunk/src/testcases/org/apache/poi/hssf/usermodel/TestFormulaEvaluatorBugs.java (original)
+++ poi/trunk/src/testcases/org/apache/poi/hssf/usermodel/TestFormulaEvaluatorBugs.java Thu
Sep  4 16:16:15 2008
@@ -19,6 +19,8 @@
 
 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 @@
  */
 public final class TestFormulaEvaluatorBugs extends TestCase {
 
+	private static final boolean OUTPUT_TEST_FILES = false;
 	private String tmpDirName;
 
 	protected void setUp() {
@@ -65,13 +68,15 @@
 		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 @@
 		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 @@
 	}
 	
 	/**
-	 * 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



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@poi.apache.org
For additional commands, e-mail: commits-help@poi.apache.org


Mime
View raw message