svn commit: r698047 - in /poi/trunk/src: documentation/content/xdocs/ java/org/apache/poi/hssf/usermodel/ java/org/apache/poi/ss/formula/ testcases/org/apache/poi/hssf/ testcases/org/apache/poi/hssf/record/formula/eval/ testcases/org/apache/poi/hssf/us...

[jo...@apache.org]

Author: josh
Date: Mon Sep 22 17:40:22 2008
New Revision: 698047

URL: http://svn.apache.org/viewvc?rev=698047&view=rev
Log:
Optimised the FormulaEvaluator to take cell dependencies into account

Added:
    poi/trunk/src/java/org/apache/poi/ss/formula/CellCacheEntry.java
    poi/trunk/src/java/org/apache/poi/ss/formula/CellLocation.java
    poi/trunk/src/java/org/apache/poi/ss/formula/IEvaluationListener.java
    poi/trunk/src/java/org/apache/poi/ss/formula/SheetRefEvaluator.java   (contents, props changed)
      - copied, changed from r697141, poi/trunk/src/java/org/apache/poi/ss/formula/CellEvaluator.java
    poi/trunk/src/testcases/org/apache/poi/ss/formula/EvaluationListener.java
    poi/trunk/src/testcases/org/apache/poi/ss/formula/TestEvaluationCache.java
    poi/trunk/src/testcases/org/apache/poi/ss/formula/WorkbookEvaluatorTestHelper.java
Removed:
    poi/trunk/src/java/org/apache/poi/ss/formula/CellEvaluator.java
Modified:
    poi/trunk/src/documentation/content/xdocs/changes.xml
    poi/trunk/src/documentation/content/xdocs/status.xml
    poi/trunk/src/java/org/apache/poi/hssf/usermodel/HSSFEvaluationWorkbook.java
    poi/trunk/src/java/org/apache/poi/hssf/usermodel/HSSFFormulaEvaluator.java
    poi/trunk/src/java/org/apache/poi/ss/formula/CellEvaluationFrame.java
    poi/trunk/src/java/org/apache/poi/ss/formula/EvaluationCache.java
    poi/trunk/src/java/org/apache/poi/ss/formula/EvaluationTracker.java
    poi/trunk/src/java/org/apache/poi/ss/formula/EvaluationWorkbook.java
    poi/trunk/src/java/org/apache/poi/ss/formula/LazyAreaEval.java
    poi/trunk/src/java/org/apache/poi/ss/formula/LazyRefEval.java
    poi/trunk/src/java/org/apache/poi/ss/formula/WorkbookEvaluator.java
    poi/trunk/src/testcases/org/apache/poi/hssf/HSSFTests.java
    poi/trunk/src/testcases/org/apache/poi/hssf/record/formula/eval/TestCircularReferences.java
    poi/trunk/src/testcases/org/apache/poi/hssf/usermodel/TestFormulaEvaluatorBugs.java
    poi/trunk/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFFormulaEvaluator.java

Modified: poi/trunk/src/documentation/content/xdocs/changes.xml
URL: http://svn.apache.org/viewvc/poi/trunk/src/documentation/content/xdocs/changes.xml?rev=698047&r1=698046&r2=698047&view=diff
==============================================================================
--- poi/trunk/src/documentation/content/xdocs/changes.xml (original)
+++ poi/trunk/src/documentation/content/xdocs/changes.xml Mon Sep 22 17:40:22 2008
@@ -37,6 +37,7 @@
 
 		<!-- Don't forget to update status.xml too! -->
         <release version="3.2-alpha1" date="2008-??-??">
+           <action dev="POI-DEVELOPERS" type="add">Optimised the FormulaEvaluator to take cell dependencies into account</action>
            <action dev="POI-DEVELOPERS" type="add">16936 - Initial support for whole-row cell styling</action>
            <action dev="POI-DEVELOPERS" type="add">Update hssf.extractor.ExcelExtractor to optionally output blank cells too</action>
            <action dev="POI-DEVELOPERS" type="add">Include the sheet name in the output of examples.XLS2CSVmra</action>

Modified: poi/trunk/src/documentation/content/xdocs/status.xml
URL: http://svn.apache.org/viewvc/poi/trunk/src/documentation/content/xdocs/status.xml?rev=698047&r1=698046&r2=698047&view=diff
==============================================================================
--- poi/trunk/src/documentation/content/xdocs/status.xml (original)
+++ poi/trunk/src/documentation/content/xdocs/status.xml Mon Sep 22 17:40:22 2008
@@ -34,6 +34,7 @@
 	<!-- Don't forget to update changes.xml too! -->
     <changes>
         <release version="3.2-alpha1" date="2008-??-??">
+           <action dev="POI-DEVELOPERS" type="add">Optimised the FormulaEvaluator to take cell dependencies into account</action>
            <action dev="POI-DEVELOPERS" type="add">16936 - Initial support for whole-row cell styling</action>
            <action dev="POI-DEVELOPERS" type="add">Update hssf.extractor.ExcelExtractor to optionally output blank cells too</action>
            <action dev="POI-DEVELOPERS" type="add">Include the sheet name in the output of examples.XLS2CSVmra</action>

Modified: poi/trunk/src/java/org/apache/poi/hssf/usermodel/HSSFEvaluationWorkbook.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/hssf/usermodel/HSSFEvaluationWorkbook.java?rev=698047&r1=698046&r2=698047&view=diff
==============================================================================
--- poi/trunk/src/java/org/apache/poi/hssf/usermodel/HSSFEvaluationWorkbook.java (original)
+++ poi/trunk/src/java/org/apache/poi/hssf/usermodel/HSSFEvaluationWorkbook.java Mon Sep 22 17:40:22 2008
@@ -71,11 +71,9 @@
 	public HSSFSheet getSheet(int sheetIndex) {
 		return _uBook.getSheetAt(sheetIndex);
 	}
-
-	public HSSFSheet getSheetByExternSheetIndex(int externSheetIndex) {
-		int sheetIndex = _iBook.getSheetIndexFromExternSheetIndex(externSheetIndex);
-		return _uBook.getSheetAt(sheetIndex);
-	}
+	public int convertFromExternSheetIndex(int externSheetIndex) {
+		return _iBook.getSheetIndexFromExternSheetIndex(externSheetIndex);
+}
 
 	public HSSFWorkbook getWorkbook() {
 		return _uBook;

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=698047&r1=698046&r2=698047&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 Mon Sep 22 17:40:22 2008
@@ -19,6 +19,7 @@
 
 import java.util.Iterator;
 
+import org.apache.poi.hssf.record.formula.eval.BlankEval;
 import org.apache.poi.hssf.record.formula.eval.BoolEval;
 import org.apache.poi.hssf.record.formula.eval.ErrorEval;
 import org.apache.poi.hssf.record.formula.eval.NumberEval;
@@ -50,14 +51,7 @@
 		}
 	}
 	public HSSFFormulaEvaluator(HSSFWorkbook workbook) {
-   		_bookEvaluator = new WorkbookEvaluator(HSSFEvaluationWorkbook.create(workbook));
-	}
-
-	/**
-	 * TODO for debug/test use
-	 */
-	/* package */ int getEvaluationCount() {
-		return _bookEvaluator.getEvaluationCount();
+		_bookEvaluator = new WorkbookEvaluator(HSSFEvaluationWorkbook.create(workbook));
 	}
 
 	/**
@@ -81,12 +75,23 @@
 		_bookEvaluator.clearAllCachedResultValues();
 	}
 	/**
+	 * Sets the cached value for a plain (non-formula) cell.
 	 * Should be called whenever there are changes to individual 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
+	 * @param never <code>null</code>. Use {@link BlankEval#INSTANCE} when the cell is being 
+	 * cleared. Otherwise an instance of {@link NumberEval}, {@link StringEval}, {@link BoolEval}
+	 * or {@link ErrorEval} to represent a plain cell value.
 	 */
-	public void clearCachedResultValue(HSSFSheet sheet, int rowIndex, int columnIndex) {
-		_bookEvaluator.clearCachedResultValue(sheet, rowIndex, columnIndex);
+	public void setCachedPlainValue(HSSFSheet sheet, int rowIndex, int columnIndex, ValueEval value) {
+		_bookEvaluator.setCachedPlainValue(sheet, rowIndex, columnIndex, value);
+	}
+	/**
+	 * Should be called to tell the cell value cache that the specified cell has just become a
+	 * formula cell, or the formula text has changed 
+	 */
+	public void notifySetFormula(HSSFSheet sheet, int rowIndex, int columnIndex) {
+		_bookEvaluator.notifySetFormula(sheet, rowIndex, columnIndex);
 	}
 
 	/**
@@ -95,7 +100,9 @@
 	 * the cell and also its cell type. This method should be preferred over
 	 * evaluateInCell() when the call should not modify the contents of the
 	 * original cell.
-	 * @param cell
+	 * 
+	 * @param cell may be <code>null</code> signifying that the cell is not present (or blank)
+	 * @return <code>null</code> if the supplied cell is <code>null</code> or blank
 	 */
 	public CellValue evaluate(HSSFCell cell) {
 		if (cell == null) {
@@ -113,6 +120,8 @@
 				return new CellValue(cell.getNumericCellValue());
 			case HSSFCell.CELL_TYPE_STRING:
 				return new CellValue(cell.getRichStringCellValue().getString());
+			case HSSFCell.CELL_TYPE_BLANK:
+				return null;
 		}
 		throw new IllegalStateException("Bad cell type (" + cell.getCellType() + ")");
 	}

Added: poi/trunk/src/java/org/apache/poi/ss/formula/CellCacheEntry.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/ss/formula/CellCacheEntry.java?rev=698047&view=auto
==============================================================================
--- poi/trunk/src/java/org/apache/poi/ss/formula/CellCacheEntry.java (added)
+++ poi/trunk/src/java/org/apache/poi/ss/formula/CellCacheEntry.java Mon Sep 22 17:40:22 2008
@@ -0,0 +1,114 @@
+/* ====================================================================
+   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.ss.formula;
+
+import java.util.HashSet;
+import java.util.Set;
+
+import org.apache.poi.hssf.record.formula.eval.BoolEval;
+import org.apache.poi.hssf.record.formula.eval.ErrorEval;
+import org.apache.poi.hssf.record.formula.eval.NumberEval;
+import org.apache.poi.hssf.record.formula.eval.StringEval;
+import org.apache.poi.hssf.record.formula.eval.ValueEval;
+
+/**
+ * Stores the parameters that identify the evaluation of one cell.<br/>
+ */
+final class CellCacheEntry {
+	private ValueEval _value;
+	private final Set _consumingCells;
+	private CellLocation[] _usedCells;
+	private boolean _isPlainValueCell;
+	
+	public CellCacheEntry() {
+		_consumingCells = new HashSet();
+	}
+	public boolean updatePlainValue(ValueEval value) {
+		boolean wasChanged = false;
+		if (!_isPlainValueCell) {
+			wasChanged = true;
+		}
+		if (!areValuesEqual(_value, value)) {
+			wasChanged = true;
+		}
+		_isPlainValueCell = true;
+		_value = value;
+		_usedCells = null;
+		return wasChanged;
+	}
+	private boolean areValuesEqual(ValueEval a, ValueEval b) {
+		if (a == null) {
+			return false;
+		}
+		Class cls = a.getClass();
+		if (cls != b.getClass()) {
+			// value type is changing
+			return false;
+		}
+		if (cls == NumberEval.class) {
+			return ((NumberEval)a).getNumberValue() == ((NumberEval)b).getNumberValue();
+		}
+		if (cls == StringEval.class) {
+			return ((StringEval)a).getStringValue().equals(((StringEval)b).getStringValue());
+		}
+		if (cls == BoolEval.class) {
+			return ((BoolEval)a).getBooleanValue() == ((BoolEval)b).getBooleanValue();
+		}
+		if (cls == ErrorEval.class) {
+			return ((ErrorEval)a).getErrorCode() == ((ErrorEval)b).getErrorCode();
+		}
+		throw new IllegalStateException("Unexpected value class (" + cls.getName() + ")");
+	}
+	public void setFormulaResult(ValueEval value, CellLocation[] usedCells) {
+		_isPlainValueCell = false;
+		_value = value;
+		_usedCells = usedCells;
+	}
+	public void addConsumingCell(CellLocation cellLoc) {
+		_consumingCells.add(cellLoc);
+		
+	}
+	public CellLocation[] getConsumingCells() {
+		int nItems = _consumingCells.size();
+		if (nItems < 1) {
+			return CellLocation.EMPTY_ARRAY;
+		}
+		CellLocation[] result = new CellLocation[nItems];
+		_consumingCells.toArray(result);
+		return result;
+	}
+	public CellLocation[] getUsedCells() {
+		return _usedCells;
+	}
+	public void clearConsumingCell(CellLocation fc) {
+		if(!_consumingCells.remove(fc)) {
+			throw new IllegalStateException("Cell '" + fc.formatAsString() + "' does not use this cell");
+		}
+		
+	}
+	public ValueEval getValue() {
+		ValueEval result = _value;
+		if (result == null) {
+			if (_isPlainValueCell) {
+				throw new IllegalStateException("Plain value cell should always have a value");
+				
+			}
+		}
+		return result;
+	}
+}
\ No newline at end of file

Modified: poi/trunk/src/java/org/apache/poi/ss/formula/CellEvaluationFrame.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/ss/formula/CellEvaluationFrame.java?rev=698047&r1=698046&r2=698047&view=diff
==============================================================================
--- poi/trunk/src/java/org/apache/poi/ss/formula/CellEvaluationFrame.java (original)
+++ poi/trunk/src/java/org/apache/poi/ss/formula/CellEvaluationFrame.java Mon Sep 22 17:40:22 2008
@@ -17,55 +17,47 @@
 
 package org.apache.poi.ss.formula;
 
+import java.util.HashSet;
+import java.util.Set;
+
 /**
- * Stores the parameters that identify the evaluation of one cell.<br/>
+ * Stores details about the current evaluation of a cell.<br/>
  */
 final class CellEvaluationFrame {
 
-	private final int _sheetIndex;
-	private final int _srcRowNum;
-	private final int _srcColNum;
-	private final int _hashCode;
-
-	public CellEvaluationFrame(int sheetIndex, int srcRowNum, int srcColNum) {
-		if (sheetIndex < 0) {
-			throw new IllegalArgumentException("sheetIndex must not be negative");
-		}
-		_sheetIndex = sheetIndex;
-		_srcRowNum = srcRowNum;
-		_srcColNum = srcColNum;
-		_hashCode = sheetIndex + 17 * (srcRowNum + 17 * srcColNum);
-	}
+	private final CellLocation _cellLocation;
+	private final Set _usedCells;
 
-	public boolean equals(Object obj) {
-		CellEvaluationFrame other = (CellEvaluationFrame) obj;
-		if (_sheetIndex != other._sheetIndex) {
-			return false;
-		}
-		if (_srcRowNum != other._srcRowNum) {
-			return false;
-		}
-		if (_srcColNum != other._srcColNum) {
-			return false;
-		}
-		return true;
+	public CellEvaluationFrame(CellLocation cellLoc) {
+		_cellLocation = cellLoc;
+		_usedCells = new HashSet();
 	}
-	public int hashCode() {
-		return _hashCode;
-	}
-
-	/**
-	 * @return human readable string for debug purposes
-	 */
-	public String formatAsString() {
-		return "R=" + _srcRowNum + " C=" + _srcColNum + " ShIx=" + _sheetIndex;
+	public CellLocation getCoordinates() {
+		return _cellLocation;
 	}
 
 	public String toString() {
 		StringBuffer sb = new StringBuffer(64);
 		sb.append(getClass().getName()).append(" [");
-		sb.append(formatAsString());
+		sb.append(_cellLocation.formatAsString());
 		sb.append("]");
 		return sb.toString();
 	}
+	public void addUsedCell(CellLocation coordinates) {
+		_usedCells.add(coordinates);
+	}
+	/**
+	 * @return never <code>null</code>, (possibly empty) array of all cells directly used while 
+	 * evaluating the formula of this frame.  For non-formula cells this will always be an empty
+	 * array;
+	 */
+	public CellLocation[] getUsedCells() {
+		int nItems = _usedCells.size();
+		if (nItems < 1) {
+			return CellLocation.EMPTY_ARRAY;
+		}
+		CellLocation[] result = new CellLocation[nItems];
+		_usedCells.toArray(result);
+		return result;
+	}
 }
\ No newline at end of file

Added: poi/trunk/src/java/org/apache/poi/ss/formula/CellLocation.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/ss/formula/CellLocation.java?rev=698047&view=auto
==============================================================================
--- poi/trunk/src/java/org/apache/poi/ss/formula/CellLocation.java (added)
+++ poi/trunk/src/java/org/apache/poi/ss/formula/CellLocation.java Mon Sep 22 17:40:22 2008
@@ -0,0 +1,81 @@
+/* ====================================================================
+   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.ss.formula;
+
+/**
+ * Stores the parameters that identify the evaluation of one cell.<br/>
+ */
+final class CellLocation {
+	public static final CellLocation[] EMPTY_ARRAY = { };
+	
+	private final int _sheetIndex;
+	private final int _rowIndex;
+	private final int _columnIndex;
+	private final int _hashCode;
+
+	public CellLocation(int sheetIndex, int rowIndex, int columnIndex) {
+		if (sheetIndex < 0) {
+			throw new IllegalArgumentException("sheetIndex must not be negative");
+		}
+		_sheetIndex = sheetIndex;
+		_rowIndex = rowIndex;
+		_columnIndex = columnIndex;
+		_hashCode = sheetIndex + 17 * (rowIndex + 17 * columnIndex);
+	}
+	public int getSheetIndex() {
+		return _sheetIndex;
+	}
+	public int getRowIndex() {
+		return _rowIndex;
+	}
+	public int getColumnIndex() {
+		return _columnIndex;
+	}
+
+	public boolean equals(Object obj) {
+		CellLocation other = (CellLocation) obj;
+		if (getSheetIndex() != other.getSheetIndex()) {
+			return false;
+		}
+		if (getRowIndex() != other.getRowIndex()) {
+			return false;
+		}
+		if (getColumnIndex() != other.getColumnIndex()) {
+			return false;
+		}
+		return true;
+	}
+	public int hashCode() {
+		return _hashCode;
+	}
+
+	/**
+	 * @return human readable string for debug purposes
+	 */
+	public String formatAsString() {
+		return  "ShIx=" + getSheetIndex() + " R=" + getRowIndex() + " C=" + getColumnIndex();
+	}
+
+	public String toString() {
+		StringBuffer sb = new StringBuffer(64);
+		sb.append(getClass().getName()).append(" [");
+		sb.append(formatAsString());
+		sb.append("]");
+		return sb.toString();
+	}
+}
\ No newline at end of file

Modified: poi/trunk/src/java/org/apache/poi/ss/formula/EvaluationCache.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/ss/formula/EvaluationCache.java?rev=698047&r1=698046&r2=698047&view=diff
==============================================================================
--- poi/trunk/src/java/org/apache/poi/ss/formula/EvaluationCache.java (original)
+++ poi/trunk/src/java/org/apache/poi/ss/formula/EvaluationCache.java Mon Sep 22 17:40:22 2008
@@ -17,9 +17,13 @@
 
 package org.apache.poi.ss.formula;
 
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Comparator;
 import java.util.HashMap;
-import java.util.List;
+import java.util.HashSet;
 import java.util.Map;
+import java.util.Set;
 
 import org.apache.poi.hssf.record.formula.eval.ValueEval;
 import org.apache.poi.hssf.usermodel.HSSFFormulaEvaluator;
@@ -34,66 +38,176 @@
  */
 final class EvaluationCache {
 
-	private static final CellEvaluationFrame[] EMPTY_CEF_ARRAY = { };
-	private final Map _valuesByKey;
-	private final Map _consumingCellsByDest;
-
-	/* package */EvaluationCache() {
-		_valuesByKey = new HashMap();
-		_consumingCellsByDest = new HashMap();
+	private final Map _entriesByLocation;
+	private final Map _consumingCellsByUsedCells;
+	/** only used for testing. <code>null</code> otherwise */
+	private final IEvaluationListener _evaluationListener;
+
+	/* package */EvaluationCache(IEvaluationListener evaluationListener) {
+		_evaluationListener = evaluationListener;
+		_entriesByLocation = new HashMap();
+		_consumingCellsByUsedCells = new HashMap();
 	}
 
-	public ValueEval getValue(int sheetIndex, int srcRowNum, int srcColNum) {
-		return getValue(new CellEvaluationFrame(sheetIndex, srcRowNum, srcColNum));
+	/**
+	 * @param cellLoc never <code>null</code>
+	 * @return only ever <code>null</code> for formula cells that have had their cached value cleared
+	 */
+	public ValueEval getValue(CellLocation cellLoc) {
+		return getEntry(cellLoc).getValue();
 	}
 
-	/* package */ ValueEval getValue(CellEvaluationFrame key) {
-		return (ValueEval) _valuesByKey.get(key);
+	/**
+	 * @param cellLoc
+	 * @param usedCells never <code>null</code>, (possibly zero length) array of all cells actually 
+	 * directly used when evaluating the formula
+	 * @param isPlainValue pass <code>true</code> if cellLoc refers to a plain value (non-formula) 
+	 * cell, <code>false</code> for a formula cell.
+	 * @param value the value of a non-formula cell or the result of evaluating the cell formula
+	 * Pass <code>null</code> to signify clearing the cached result of a formula cell) 
+	 */
+	public void setValue(CellLocation cellLoc, boolean isPlainValue, 
+			CellLocation[] usedCells, ValueEval value) {
+
+		CellCacheEntry existingEntry = (CellCacheEntry) _entriesByLocation.get(cellLoc);
+		if (existingEntry != null && existingEntry.getValue() != null) {
+			if (isPlainValue) {
+				// can set a plain cell cached value any time
+			} else if (value == null) {
+				// or clear the cached value of a formula cell at any time
+			} else {
+				// but a formula cached value would only be getting updated if the cache didn't have a value to start with
+				throw new IllegalStateException("Already have cached value for this cell: "
+						+ cellLoc.formatAsString());
+			}
+		}
+		if (_evaluationListener == null) {
+			// optimisation - don't bother sorting if there is no listener.
+		} else {
+			// for testing
+			// make order of callbacks to listener more deterministic
+			Arrays.sort(usedCells, CellLocationComparator);
+		}
+		CellCacheEntry entry = getEntry(cellLoc);
+		CellLocation[] consumingFormulaCells = entry.getConsumingCells();
+		CellLocation[] prevUsedCells = entry.getUsedCells();
+		if (isPlainValue) {
+			
+			if(!entry.updatePlainValue(value)) {
+				return;
+			}
+		} else {
+			entry.setFormulaResult(value, usedCells);
+			for (int i = 0; i < usedCells.length; i++) {
+				getEntry(usedCells[i]).addConsumingCell(cellLoc);
+			}
+		}
+		// need to tell all cells that were previously used, but no longer are, 
+		// that they are not consumed by this cell any more
+		unlinkConsumingCells(prevUsedCells, usedCells, cellLoc);
+		
+		// clear all cells that directly or indirectly depend on this one
+		recurseClearCachedFormulaResults(consumingFormulaCells, 0);
 	}
 
-	public void setValue(int sheetIndex, int srcRowNum, int srcColNum, ValueEval value) {
-		setValue(new CellEvaluationFrame(sheetIndex, srcRowNum, srcColNum), value);
+	private void unlinkConsumingCells(CellLocation[] prevUsedCells, CellLocation[] usedCells,
+			CellLocation cellLoc) {
+		if (prevUsedCells == null) {
+			return;
+		}
+		int nPrevUsed = prevUsedCells.length;
+		if (nPrevUsed < 1) {
+			return;
+		}
+		int nUsed = usedCells.length;
+		Set usedSet;
+		if (nUsed < 1) {
+			usedSet = Collections.EMPTY_SET;
+		} else {
+			usedSet = new HashSet(nUsed * 3 / 2);
+			for (int i = 0; i < nUsed; i++) {
+				usedSet.add(usedCells[i]);
+			}
+		}
+		for (int i = 0; i < nPrevUsed; i++) {
+			CellLocation prevUsed = prevUsedCells[i];
+			if (!usedSet.contains(prevUsed)) {
+				// previously was used by cellLoc, but not anymore
+				getEntry(prevUsed).clearConsumingCell(cellLoc);
+				if (_evaluationListener != null) {
+					//TODO _evaluationListener.onUnconsume(prevUsed.getSheetIndex(), etc)
+				}
+			}
+		}
+		
 	}
 
-	/* package */ void setValue(CellEvaluationFrame key, ValueEval value) {
-		if (_valuesByKey.containsKey(key)) {
-			throw new RuntimeException("Already have cached value for this cell");
+	/**
+	 * Calls formulaCell.setFormulaResult(null, null) recursively all the way up the tree of 
+	 * dependencies. Calls usedCell.clearConsumingCell(fc) for each child of a cell that is
+	 * cleared along the way.
+	 * @param formulaCells
+	 */
+	private void recurseClearCachedFormulaResults(CellLocation[] formulaCells, int depth) {
+		int nextDepth = depth+1;
+		for (int i = 0; i < formulaCells.length; i++) {
+			CellLocation fc = formulaCells[i];
+			CellCacheEntry formulaCell = getEntry(fc);
+			CellLocation[] usedCells = formulaCell.getUsedCells();
+			if (usedCells != null) {
+				for (int j = 0; j < usedCells.length; j++) {
+					CellCacheEntry usedCell = getEntry(usedCells[j]);
+					usedCell.clearConsumingCell(fc);
+				}
+			}
+			if (_evaluationListener != null) {
+				ValueEval value = formulaCell.getValue();
+				_evaluationListener.onClearDependentCachedValue(fc.getSheetIndex(), fc.getRowIndex(), fc.getColumnIndex(), value, nextDepth);
+			}
+			formulaCell.setFormulaResult(null, null);
+			recurseClearCachedFormulaResults(formulaCell.getConsumingCells(), nextDepth);
 		}
-		_valuesByKey.put(key, value);
 	}
 
+	private CellCacheEntry getEntry(CellLocation cellLoc) {
+		CellCacheEntry result = (CellCacheEntry)_entriesByLocation.get(cellLoc);
+		if (result == null) {
+			result = new CellCacheEntry();
+			_entriesByLocation.put(cellLoc, result);
+		}
+		return result;
+	}
 	/**
 	 * Should be called whenever there are changes to input cells in the evaluated workbook.
 	 */
 	public void clear() {
-		_valuesByKey.clear();
+		if(_evaluationListener != null) {
+			_evaluationListener.onClearWholeCache();
+		}
+		_entriesByLocation.clear();
+		_consumingCellsByUsedCells.clear();
 	}
 
-	public void clearValue(int sheetIndex, int rowIndex, int columnIndex) {
-		clearValuesRecursive(new CellEvaluationFrame(sheetIndex, rowIndex, columnIndex));
-		
-	}
 
-	private void clearValuesRecursive(CellEvaluationFrame cef) {
-		CellEvaluationFrame[] consumingCells = getConsumingCells(cef);
-		for (int i = 0; i < consumingCells.length; i++) {
-			clearValuesRecursive(consumingCells[i]);
-		}
-		_valuesByKey.remove(cef);
-		_consumingCellsByDest.remove(cef);
-	}
-
-	private CellEvaluationFrame[] getConsumingCells(CellEvaluationFrame cef) {
-		List temp = (List) _consumingCellsByDest.get(cef);
-		if (temp == null) {
-			return EMPTY_CEF_ARRAY;
-		}
-		int nItems = temp.size();
-		if (temp.size() < 1) {
-			return EMPTY_CEF_ARRAY;
+	private static final Comparator CellLocationComparator = new Comparator() {
+
+		public int compare(Object a, Object b) {
+			CellLocation clA = (CellLocation) a;
+			CellLocation clB = (CellLocation) b;
+			
+			int cmp;
+			cmp = clA.getSheetIndex() - clB.getSheetIndex();
+			if (cmp != 0) {
+				return cmp;
+			}
+			cmp = clA.getRowIndex() - clB.getRowIndex();
+			if (cmp != 0) {
+				return cmp;
+			}
+			cmp = clA.getColumnIndex() - clB.getColumnIndex();
+			
+			return cmp;
 		}
-		CellEvaluationFrame[] result = new CellEvaluationFrame[nItems];
-		temp.toArray(result);
-		return result;
-	}
+		
+	};
 }

Modified: poi/trunk/src/java/org/apache/poi/ss/formula/EvaluationTracker.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/ss/formula/EvaluationTracker.java?rev=698047&r1=698046&r2=698047&view=diff
==============================================================================
--- poi/trunk/src/java/org/apache/poi/ss/formula/EvaluationTracker.java (original)
+++ poi/trunk/src/java/org/apache/poi/ss/formula/EvaluationTracker.java Mon Sep 22 17:40:22 2008
@@ -18,7 +18,9 @@
 package org.apache.poi.ss.formula;
 
 import java.util.ArrayList;
+import java.util.HashSet;
 import java.util.List;
+import java.util.Set;
 
 import org.apache.poi.hssf.record.formula.eval.ErrorEval;
 import org.apache.poi.hssf.record.formula.eval.ValueEval;
@@ -36,11 +38,13 @@
 final class EvaluationTracker {
 
 	private final List _evaluationFrames;
+	private final Set _currentlyEvaluatingCells;
 	private final EvaluationCache _cache;
 
 	public EvaluationTracker(EvaluationCache cache) {
 		_cache = cache;
 		_evaluationFrames = new ArrayList();
+		_currentlyEvaluatingCells = new HashSet();
 	}
 
 	/**
@@ -58,14 +62,17 @@
 	 * @return <code>true</code> if the specified cell has not been visited yet in the current 
 	 * evaluation. <code>false</code> if the specified cell is already being evaluated.
 	 */
-	public ValueEval startEvaluate(int sheetIndex, int srcRowNum, int srcColNum) {
-		CellEvaluationFrame cef = new CellEvaluationFrame(sheetIndex, srcRowNum, srcColNum);
-		if (_evaluationFrames.contains(cef)) {
+	public ValueEval startEvaluate(CellLocation cellLoc) {
+		if (cellLoc == null) {
+			throw new IllegalArgumentException("cellLoc must not be null");
+		}
+		if (_currentlyEvaluatingCells.contains(cellLoc)) {
 			return ErrorEval.CIRCULAR_REF_ERROR;
 		}
-		ValueEval result = _cache.getValue(cef);
+		ValueEval result = _cache.getValue(cellLoc);
 		if (result == null) {
-			_evaluationFrames.add(cef);
+			_currentlyEvaluatingCells.add(cellLoc);
+			_evaluationFrames.add(new CellEvaluationFrame(cellLoc));
 		}
 		return result;
 	}
@@ -83,24 +90,41 @@
 	 * and form more meaningful error messages.
 	 * @param result 
 	 */
-	public void endEvaluate(int sheetIndex, int srcRowNum, int srcColNum, ValueEval result) {
+	public void endEvaluate(CellLocation cellLoc, ValueEval result, boolean isPlainValueCell) {
+
 		int nFrames = _evaluationFrames.size();
 		if (nFrames < 1) {
 			throw new IllegalStateException("Call to endEvaluate without matching call to startEvaluate");
 		}
 
 		nFrames--;
-		CellEvaluationFrame cefExpected = (CellEvaluationFrame) _evaluationFrames.get(nFrames);
-		CellEvaluationFrame cefActual = new CellEvaluationFrame(sheetIndex, srcRowNum, srcColNum);
-		if (!cefActual.equals(cefExpected)) {
+		CellEvaluationFrame frame = (CellEvaluationFrame) _evaluationFrames.get(nFrames);
+		CellLocation coordinates = frame.getCoordinates();
+		if (!coordinates.equals(cellLoc)) {
 			throw new RuntimeException("Wrong cell specified. "
 					+ "Corresponding startEvaluate() call was for cell {"
-					+ cefExpected.formatAsString() + "} this endEvaluate() call is for cell {"
-					+ cefActual.formatAsString() + "}");
+					+ coordinates.formatAsString() + "} this endEvaluate() call is for cell {"
+					+ cellLoc.formatAsString() + "}");
 		}
 		// else - no problems so pop current frame 
 		_evaluationFrames.remove(nFrames);
-		
-		_cache.setValue(cefActual, result);
+		_currentlyEvaluatingCells.remove(coordinates);
+
+		// TODO - don't cache results of volatile formulas 
+		_cache.setValue(coordinates, isPlainValueCell, frame.getUsedCells(), result);
+	}
+	/**
+	 * Tells the currently evaluating cell frame that it has a dependency on the specified 
+	 * <tt>usedCell<tt> 
+	 * @param usedCell location of cell which is referenced (and used) by the current cell.
+	 */
+	public void acceptDependency(CellLocation usedCell) {
+		int prevFrameIndex = _evaluationFrames.size()-1;
+		if (prevFrameIndex < 0) {
+			throw new IllegalStateException("Call to acceptDependency without prior call to startEvaluate");
+		}
+		CellEvaluationFrame consumingFrame = (CellEvaluationFrame) _evaluationFrames.get(prevFrameIndex);
+		consumingFrame.addUsedCell(usedCell);
 	}
+
 }

Modified: poi/trunk/src/java/org/apache/poi/ss/formula/EvaluationWorkbook.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/ss/formula/EvaluationWorkbook.java?rev=698047&r1=698046&r2=698047&view=diff
==============================================================================
--- poi/trunk/src/java/org/apache/poi/ss/formula/EvaluationWorkbook.java (original)
+++ poi/trunk/src/java/org/apache/poi/ss/formula/EvaluationWorkbook.java Mon Sep 22 17:40:22 2008
@@ -35,7 +35,7 @@
 
 	HSSFSheet getSheet(int sheetIndex);
 
-	HSSFSheet getSheetByExternSheetIndex(int externSheetIndex);
+	int convertFromExternSheetIndex(int externSheetIndex);
 	EvaluationName getName(NamePtg namePtg);
 	String resolveNameXText(NameXPtg ptg);
 	Ptg[] getFormulaTokens(HSSFCell cell);

Added: poi/trunk/src/java/org/apache/poi/ss/formula/IEvaluationListener.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/ss/formula/IEvaluationListener.java?rev=698047&view=auto
==============================================================================
--- poi/trunk/src/java/org/apache/poi/ss/formula/IEvaluationListener.java (added)
+++ poi/trunk/src/java/org/apache/poi/ss/formula/IEvaluationListener.java Mon Sep 22 17:40:22 2008
@@ -0,0 +1,39 @@
+/* ====================================================================
+   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.ss.formula;
+
+import org.apache.poi.hssf.record.formula.Ptg;
+import org.apache.poi.hssf.record.formula.eval.ValueEval;
+
+/**
+ * Tests can implement this class to track the internal working of the {@link WorkbookEvaluator}.<br/>
+ * 
+ * For POI internal testing use only
+ * 
+ * @author Josh Micich
+ */
+interface IEvaluationListener {
+
+	void onCacheHit(int sheetIndex, int rowIndex, int columnIndex, ValueEval result);
+	void onReadPlainValue(int sheetIndex, int srcRowNum, int srcColNum, ValueEval value);
+	void onStartEvaluate(int sheetIndex, int rowIndex, int columnIndex, Ptg[] ptgs);
+	void onEndEvaluate(int sheetIndex, int rowIndex, int columnIndex, ValueEval result);
+	void onClearWholeCache();
+	void onClearCachedValue(int sheetIndex, int rowIndex, int columnIndex, ValueEval value);
+	void onClearDependentCachedValue(int sheetIndex, int rowIndex, int columnIndex, ValueEval value, int depth);
+}

Modified: poi/trunk/src/java/org/apache/poi/ss/formula/LazyAreaEval.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/ss/formula/LazyAreaEval.java?rev=698047&r1=698046&r2=698047&view=diff
==============================================================================
--- poi/trunk/src/java/org/apache/poi/ss/formula/LazyAreaEval.java (original)
+++ poi/trunk/src/java/org/apache/poi/ss/formula/LazyAreaEval.java Mon Sep 22 17:40:22 2008
@@ -21,11 +21,7 @@
 import org.apache.poi.hssf.record.formula.AreaI.OffsetArea;
 import org.apache.poi.hssf.record.formula.eval.AreaEval;
 import org.apache.poi.hssf.record.formula.eval.AreaEvalBase;
-import org.apache.poi.hssf.record.formula.eval.BlankEval;
 import org.apache.poi.hssf.record.formula.eval.ValueEval;
-import org.apache.poi.hssf.usermodel.HSSFCell;
-import org.apache.poi.hssf.usermodel.HSSFRow;
-import org.apache.poi.hssf.usermodel.HSSFSheet;
 import org.apache.poi.hssf.util.CellReference;
 
 /**
@@ -34,12 +30,10 @@
  */
 final class LazyAreaEval extends AreaEvalBase {
 
-	private final HSSFSheet _sheet;
-	private final CellEvaluator _evaluator;
+	private final SheetRefEvaluator _evaluator;
 
-	public LazyAreaEval(AreaI ptg, HSSFSheet sheet, CellEvaluator evaluator) {
+	public LazyAreaEval(AreaI ptg, SheetRefEvaluator evaluator) {
 		super(ptg);
-		_sheet = sheet;
 		_evaluator = evaluator;
 	}
 
@@ -48,30 +42,21 @@
 		int rowIx = (relativeRowIndex + getFirstRow() ) & 0xFFFF;
 		int colIx = (relativeColumnIndex + getFirstColumn() ) & 0x00FF;
 		
-		HSSFRow row = _sheet.getRow(rowIx);
-		if (row == null) {
-			return BlankEval.INSTANCE;
-		}
-		HSSFCell cell = row.getCell(colIx);
-		if (cell == null) {
-			return BlankEval.INSTANCE;
-		}
-		return _evaluator.getEvalForCell(cell);
+		return _evaluator.getEvalForCell(rowIx, colIx);
 	}
 
 	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, _evaluator);
+		return new LazyAreaEval(area, _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 = _evaluator.getSheetName(_sheet);
-		sb.append(sheetName);
+		sb.append(_evaluator.getSheetName());
 		sb.append('!');
 		sb.append(crA.formatAsString());
 		sb.append(':');

Modified: poi/trunk/src/java/org/apache/poi/ss/formula/LazyRefEval.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/ss/formula/LazyRefEval.java?rev=698047&r1=698046&r2=698047&view=diff
==============================================================================
--- poi/trunk/src/java/org/apache/poi/ss/formula/LazyRefEval.java (original)
+++ poi/trunk/src/java/org/apache/poi/ss/formula/LazyRefEval.java Mon Sep 22 17:40:22 2008
@@ -22,12 +22,8 @@
 import org.apache.poi.hssf.record.formula.RefPtg;
 import org.apache.poi.hssf.record.formula.AreaI.OffsetArea;
 import org.apache.poi.hssf.record.formula.eval.AreaEval;
-import org.apache.poi.hssf.record.formula.eval.BlankEval;
 import org.apache.poi.hssf.record.formula.eval.RefEvalBase;
 import org.apache.poi.hssf.record.formula.eval.ValueEval;
-import org.apache.poi.hssf.usermodel.HSSFCell;
-import org.apache.poi.hssf.usermodel.HSSFRow;
-import org.apache.poi.hssf.usermodel.HSSFSheet;
 import org.apache.poi.hssf.util.CellReference;
 
 /**
@@ -36,34 +32,19 @@
 */
 final class LazyRefEval extends RefEvalBase {
 
-	private final HSSFSheet _sheet;
-	private final CellEvaluator _evaluator;
+	private final SheetRefEvaluator _evaluator;
 
-
-	public LazyRefEval(RefPtg ptg, HSSFSheet sheet, CellEvaluator evaluator) {
+	public LazyRefEval(RefPtg ptg, SheetRefEvaluator sre) {
 		super(ptg.getRow(), ptg.getColumn());
-		_sheet = sheet;
-		_evaluator = evaluator;
+		_evaluator = sre;
 	}
-	public LazyRefEval(Ref3DPtg ptg, HSSFSheet sheet, CellEvaluator evaluator) {
+	public LazyRefEval(Ref3DPtg ptg, SheetRefEvaluator sre) {
 		super(ptg.getRow(), ptg.getColumn());
-		_sheet = sheet;
-		_evaluator = evaluator;
+		_evaluator = sre;
 	}
 
 	public ValueEval getInnerValueEval() {
-		int rowIx = getRow();
-		int colIx = getColumn();
-		
-		HSSFRow row = _sheet.getRow(rowIx);
-		if (row == null) {
-			return BlankEval.INSTANCE;
-		}
-		HSSFCell cell = row.getCell(colIx);
-		if (cell == null) {
-			return BlankEval.INSTANCE;
-		}
-		return _evaluator.getEvalForCell(cell);
+		return _evaluator.getEvalForCell(getRow(), getColumn());
 	}
 	
 	public AreaEval offset(int relFirstRowIx, int relLastRowIx, int relFirstColIx, int relLastColIx) {
@@ -71,15 +52,14 @@
 		AreaI area = new OffsetArea(getRow(), getColumn(),
 				relFirstRowIx, relLastRowIx, relFirstColIx, relLastColIx);
 
-		return new LazyAreaEval(area, _sheet, _evaluator);
+		return new LazyAreaEval(area, _evaluator);
 	}
 	
 	public String toString() {
 		CellReference cr = new CellReference(getRow(), getColumn());
 		StringBuffer sb = new StringBuffer();
 		sb.append(getClass().getName()).append("[");
-		String sheetName = _evaluator.getSheetName(_sheet);
-		sb.append(sheetName);
+		sb.append(_evaluator.getSheetName());
 		sb.append('!');
 		sb.append(cr.formatAsString());
 		sb.append("]");

Copied: poi/trunk/src/java/org/apache/poi/ss/formula/SheetRefEvaluator.java (from r697141, poi/trunk/src/java/org/apache/poi/ss/formula/CellEvaluator.java)
URL: http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/ss/formula/SheetRefEvaluator.java?p2=poi/trunk/src/java/org/apache/poi/ss/formula/SheetRefEvaluator.java&p1=poi/trunk/src/java/org/apache/poi/ss/formula/CellEvaluator.java&r1=697141&r2=698047&rev=698047&view=diff
==============================================================================
--- poi/trunk/src/java/org/apache/poi/ss/formula/CellEvaluator.java (original)
+++ poi/trunk/src/java/org/apache/poi/ss/formula/SheetRefEvaluator.java Mon Sep 22 17:40:22 2008
@@ -1,54 +1,49 @@
+/* ====================================================================
+   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.ss.formula;
 
-import org.apache.poi.hssf.record.formula.eval.BlankEval;
-import org.apache.poi.hssf.record.formula.eval.BoolEval;
-import org.apache.poi.hssf.record.formula.eval.ErrorEval;
-import org.apache.poi.hssf.record.formula.eval.NumberEval;
-import org.apache.poi.hssf.record.formula.eval.StringEval;
 import org.apache.poi.hssf.record.formula.eval.ValueEval;
-import org.apache.poi.hssf.usermodel.HSSFCell;
 import org.apache.poi.hssf.usermodel.HSSFSheet;
-
-final class CellEvaluator {
+/**
+ * 
+ * 
+ * @author Josh Micich
+ */
+final class SheetRefEvaluator {
 
 	private final WorkbookEvaluator _bookEvaluator;
 	private final EvaluationTracker _tracker;
+	private final HSSFSheet _sheet;
+	private final int _sheetIndex;
 
-	public CellEvaluator(WorkbookEvaluator bookEvaluator, EvaluationTracker tracker) {
+	public SheetRefEvaluator(WorkbookEvaluator bookEvaluator, EvaluationTracker tracker,
+			EvaluationWorkbook _workbook, int sheetIndex) {
 		_bookEvaluator = bookEvaluator;
 		_tracker = tracker;
+		_sheet = _workbook.getSheet(sheetIndex);
+		_sheetIndex = sheetIndex;
 	}
 
-	   /**
-     * Given a cell, find its type and from that create an appropriate ValueEval
-     * impl instance and return that. Since the cell could be an external
-     * reference, we need the sheet that this belongs to.
-     * Non existent cells are treated as empty.
-     */
-	public ValueEval getEvalForCell(HSSFCell cell) {
-
-        if (cell == null) {
-            return BlankEval.INSTANCE;
-        }
-        switch (cell.getCellType()) {
-            case HSSFCell.CELL_TYPE_NUMERIC:
-                return new NumberEval(cell.getNumericCellValue());
-            case HSSFCell.CELL_TYPE_STRING:
-                return new StringEval(cell.getRichStringCellValue().getString());
-            case HSSFCell.CELL_TYPE_FORMULA:
-                return _bookEvaluator.internalEvaluate(cell, _tracker);
-            case HSSFCell.CELL_TYPE_BOOLEAN:
-                return BoolEval.valueOf(cell.getBooleanCellValue());
-            case HSSFCell.CELL_TYPE_BLANK:
-                return BlankEval.INSTANCE;
-            case HSSFCell.CELL_TYPE_ERROR:
-                return ErrorEval.valueOf(cell.getErrorCellValue());
-        }
-        throw new RuntimeException("Unexpected cell type (" + cell.getCellType() + ")");
-    }
-
-	public String getSheetName(HSSFSheet sheet) {
-        return _bookEvaluator.getSheetName(sheet);
+	public String getSheetName() {
+		return _bookEvaluator.getSheetName(_sheetIndex);
 	}
 
+	public ValueEval getEvalForCell(int rowIndex, int columnIndex) {
+		return _bookEvaluator.evaluateReference(_sheet, _sheetIndex, rowIndex, columnIndex, _tracker);
+	}
 }

Propchange: poi/trunk/src/java/org/apache/poi/ss/formula/SheetRefEvaluator.java
------------------------------------------------------------------------------
    svn:mergeinfo = 

Modified: poi/trunk/src/java/org/apache/poi/ss/formula/WorkbookEvaluator.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/ss/formula/WorkbookEvaluator.java?rev=698047&r1=698046&r2=698047&view=diff
==============================================================================
--- poi/trunk/src/java/org/apache/poi/ss/formula/WorkbookEvaluator.java (original)
+++ poi/trunk/src/java/org/apache/poi/ss/formula/WorkbookEvaluator.java Mon Sep 22 17:40:22 2008
@@ -17,6 +17,8 @@
 
 package org.apache.poi.ss.formula;
 
+import java.util.IdentityHashMap;
+import java.util.Map;
 import java.util.Stack;
 
 import org.apache.poi.hssf.record.formula.Area3DPtg;
@@ -51,6 +53,7 @@
 import org.apache.poi.hssf.record.formula.eval.StringEval;
 import org.apache.poi.hssf.record.formula.eval.ValueEval;
 import org.apache.poi.hssf.usermodel.HSSFCell;
+import org.apache.poi.hssf.usermodel.HSSFRow;
 import org.apache.poi.hssf.usermodel.HSSFSheet;
 import org.apache.poi.hssf.util.CellReference;
 
@@ -67,43 +70,28 @@
  */
 public class WorkbookEvaluator {
 
-	/**
-	 * used to track the number of evaluations
-	 */
-	private static final class Counter {
-		public int value;
-		public int depth;
-		public Counter() {
-			value = 0;
-		}
-	}
-
 	private final EvaluationWorkbook _workbook;
 	private final EvaluationCache _cache;
 
-	private Counter _evaluationCounter;
+	private final IEvaluationListener _evaluationListener;
+	private final Map _sheetIndexesBySheet;
 
 	public WorkbookEvaluator(EvaluationWorkbook workbook) {
+		this (workbook, null);
+	}
+	/* package */ WorkbookEvaluator(EvaluationWorkbook workbook, IEvaluationListener evaluationListener) {
 		_workbook = workbook;
-		_cache = new EvaluationCache();
-		_evaluationCounter = new Counter();
+		_evaluationListener = evaluationListener;
+		_cache = new EvaluationCache(evaluationListener);
+		_sheetIndexesBySheet = new IdentityHashMap();
 	}
 
 	/**
-	 * for debug use. Used in toString methods
+	 * also for debug use. Used in toString methods
 	 */
-	/* package */ String getSheetName(HSSFSheet sheet) {
-		return getSheetName(getSheetIndex(sheet));
-	}
-	private String getSheetName(int sheetIndex) {
+	/* package */ String getSheetName(int sheetIndex) {
 		return _workbook.getSheetName(sheetIndex);
 	}
-	/**
-	 * for debug/test use
-	 */
-	public int getEvaluationCount() {
-		return _evaluationCounter.value;
-	}
 
 	private static boolean isDebugLogEnabled() {
 		return false;
@@ -121,56 +109,122 @@
 	 */
 	public void clearAllCachedResultValues() {
 		_cache.clear();
+		_sheetIndexesBySheet.clear();
 	}
 
-	public void clearCachedResultValue(HSSFSheet sheet, int rowIndex, int columnIndex) {
+	/**
+	 * Sets the cached value for a plain (non-formula) cell.
+	 * @param never <code>null</code>. Use {@link BlankEval#INSTANCE} when the cell is being 
+	 * cleared. Otherwise an instance of {@link NumberEval}, {@link StringEval}, {@link BoolEval}
+	 * or {@link ErrorEval} to represent a plain cell value.
+	 */
+	public void setCachedPlainValue(HSSFSheet sheet, int rowIndex, int columnIndex, ValueEval value) {
+		if (value == null) {
+			throw new IllegalArgumentException("value must not be null");
+		}
+		int sheetIndex = getSheetIndex(sheet);
+		_cache.setValue(new CellLocation(sheetIndex, rowIndex, columnIndex), true, CellLocation.EMPTY_ARRAY, value);
+
+	}
+	/**
+	 * Should be called to tell the cell value cache that the specified cell has just become a
+	 * formula cell, or the formula text has changed 
+	 */
+	public void notifySetFormula(HSSFSheet sheet, int rowIndex, int columnIndex) {
 		int sheetIndex = getSheetIndex(sheet);
-		_cache.clearValue(sheetIndex, rowIndex, columnIndex);
+		_cache.setValue(new CellLocation(sheetIndex, rowIndex, columnIndex), false, CellLocation.EMPTY_ARRAY, null);
 
 	}
 	private int getSheetIndex(HSSFSheet sheet) {
-		// TODO cache sheet indexes too
-		return _workbook.getSheetIndex(sheet);
+		Integer result = (Integer) _sheetIndexesBySheet.get(sheet);
+		if (result == null) {
+			result = new Integer(_workbook.getSheetIndex(sheet));
+			_sheetIndexesBySheet.put(sheet, result);
+		}
+		return result.intValue();
 	}
 
 	public ValueEval evaluate(HSSFCell srcCell) {
-		return internalEvaluate(srcCell, new EvaluationTracker(_cache));
+		int sheetIndex = getSheetIndex(srcCell.getSheet());
+		CellLocation cellLoc = new CellLocation(sheetIndex, srcCell.getRowIndex(), srcCell.getCellNum());
+		return internalEvaluate(srcCell, cellLoc, new EvaluationTracker(_cache));
 	}
 
 	/**
-	 * Dev. Note: Internal evaluate must be passed only a formula cell
-	 * else a runtime exception will be thrown somewhere inside the method.
-	 * (Hence this is a private method.)
 	 * @return never <code>null</code>, never {@link BlankEval}
 	 */
-	/* package */ ValueEval internalEvaluate(HSSFCell srcCell, EvaluationTracker tracker) {
-		int srcRowNum = srcCell.getRowIndex();
-		int srcColNum = srcCell.getCellNum();
+	private ValueEval internalEvaluate(HSSFCell srcCell, CellLocation cellLoc, EvaluationTracker tracker) {
+		int sheetIndex = cellLoc.getSheetIndex();
+		int rowIndex = cellLoc.getRowIndex();
+		int columnIndex = cellLoc.getColumnIndex();
 
 		ValueEval result;
 
-		int sheetIndex = getSheetIndex(srcCell.getSheet());
-		result = tracker.startEvaluate(sheetIndex, srcRowNum, srcColNum);
+		result = tracker.startEvaluate(cellLoc);
+		IEvaluationListener evalListener = _evaluationListener;
 		if (result != null) {
+			if(evalListener != null) {
+				evalListener.onCacheHit(sheetIndex, rowIndex, columnIndex, result);
+			}
 			return result;
 		}
-		_evaluationCounter.value++;
-		_evaluationCounter.depth++;
 
+		boolean isPlainFormulaCell = false;
 		try {
-			Ptg[] ptgs = _workbook.getFormulaTokens(srcCell);
-			result = evaluateCell(sheetIndex, srcRowNum, (short)srcColNum, ptgs, tracker);
+			result = getValueFromNonFormulaCell(srcCell);
+			if (result != null) {
+				isPlainFormulaCell = true;
+				if(evalListener != null) {
+					evalListener.onReadPlainValue(sheetIndex, rowIndex, columnIndex, result);
+				}
+			} else {
+				isPlainFormulaCell = false;
+				Ptg[] ptgs = _workbook.getFormulaTokens(srcCell);
+				if(evalListener == null) {
+					result = evaluateCell(sheetIndex, rowIndex, (short)columnIndex, ptgs, tracker);
+				} else {
+					evalListener.onStartEvaluate(sheetIndex, rowIndex, columnIndex, ptgs);
+					result = evaluateCell(sheetIndex, rowIndex, (short)columnIndex, ptgs, tracker);
+					evalListener.onEndEvaluate(sheetIndex, rowIndex, columnIndex, result);
+				}
+			}
 		} finally {
-			tracker.endEvaluate(sheetIndex, srcRowNum, srcColNum, result);
-			_evaluationCounter.depth--;
+			tracker.endEvaluate(cellLoc, result, isPlainFormulaCell);
 		}
 		if (isDebugLogEnabled()) {
 			String sheetName = getSheetName(sheetIndex);
-			CellReference cr = new CellReference(srcRowNum, srcColNum);
+			CellReference cr = new CellReference(rowIndex, columnIndex);
 			logDebug("Evaluated " + sheetName + "!" + cr.formatAsString() + " to " + result.toString());
 		}
 		return result;
 	}
+	/**
+	 * Gets the value from a non-formula cell.
+	 * @param cell may be <code>null</code>
+	 * @return {@link BlankEval} if cell is <code>null</code> or blank, <code>null</code> if cell 
+	 * is a formula cell.
+	 */
+	private static ValueEval getValueFromNonFormulaCell(HSSFCell cell) {
+		if (cell == null) {
+			return BlankEval.INSTANCE;
+		}
+		int cellType = cell.getCellType();
+		switch (cellType) {
+			case HSSFCell.CELL_TYPE_FORMULA:
+				return null;
+			case HSSFCell.CELL_TYPE_NUMERIC:
+				return new NumberEval(cell.getNumericCellValue());
+			case HSSFCell.CELL_TYPE_STRING:
+				return new StringEval(cell.getRichStringCellValue().getString());
+			case HSSFCell.CELL_TYPE_BOOLEAN:
+				return BoolEval.valueOf(cell.getBooleanCellValue());
+			case HSSFCell.CELL_TYPE_BLANK:
+				return BlankEval.INSTANCE;
+			case HSSFCell.CELL_TYPE_ERROR:
+				return ErrorEval.valueOf(cell.getErrorCellValue());
+		}
+		throw new RuntimeException("Unexpected cell type (" + cellType + ")");
+	}
 	private ValueEval evaluateCell(int sheetIndex, int srcRowNum, short srcColNum, Ptg[] ptgs, EvaluationTracker tracker) {
 
 		Stack stack = new Stack();
@@ -268,10 +322,6 @@
 		return operation.evaluate(ops, srcRowNum, (short)srcColNum);
 	}
 
-	private HSSFSheet getOtherSheet(int externSheetIndex) {
-		return _workbook.getSheetByExternSheetIndex(externSheetIndex);
-	}
-
 	/**
 	 * returns an appropriate Eval impl instance for the Ptg. The Ptg must be
 	 * one of: Area3DPtg, AreaPtg, ReferencePtg, Ref3DPtg, IntPtg, NumberPtg,
@@ -311,23 +361,24 @@
 		if (ptg instanceof ErrPtg) {
 			return ErrorEval.valueOf(((ErrPtg) ptg).getErrorCode());
 		}
-		CellEvaluator ce = new CellEvaluator(this, tracker);
-		HSSFSheet sheet = _workbook.getSheet(sheetIndex);
-		if (ptg instanceof RefPtg) {
-			return new LazyRefEval(((RefPtg) ptg), sheet, ce);
-		}
-		if (ptg instanceof AreaPtg) {
-			return new LazyAreaEval(((AreaPtg) ptg), sheet, ce);
-		}
 		if (ptg instanceof Ref3DPtg) {
 			Ref3DPtg refPtg = (Ref3DPtg) ptg;
-			HSSFSheet xsheet = getOtherSheet(refPtg.getExternSheetIndex());
-			return new LazyRefEval(refPtg, xsheet, ce);
+			int otherSheetIndex = _workbook.convertFromExternSheetIndex(refPtg.getExternSheetIndex());
+			SheetRefEvaluator sre = new SheetRefEvaluator(this, tracker, _workbook, otherSheetIndex);
+			return new LazyRefEval(refPtg, sre);
 		}
 		if (ptg instanceof Area3DPtg) {
-			Area3DPtg a3dp = (Area3DPtg) ptg;
-			HSSFSheet xsheet = getOtherSheet(a3dp.getExternSheetIndex());
-			return new LazyAreaEval(a3dp, xsheet, ce);
+			Area3DPtg aptg = (Area3DPtg) ptg;
+			int otherSheetIndex = _workbook.convertFromExternSheetIndex(aptg.getExternSheetIndex());
+			SheetRefEvaluator sre = new SheetRefEvaluator(this, tracker, _workbook, otherSheetIndex);
+			return new LazyAreaEval(aptg, sre);
+		}
+		SheetRefEvaluator sre = new SheetRefEvaluator(this, tracker, _workbook, sheetIndex);
+		if (ptg instanceof RefPtg) {
+			return new LazyRefEval(((RefPtg) ptg), sre);
+		}
+		if (ptg instanceof AreaPtg) {
+			return new LazyAreaEval(((AreaPtg) ptg), sre);
 		}
 
 		if (ptg instanceof UnknownPtg) {
@@ -345,4 +396,22 @@
 		}
 		return getEvalForPtg(ptgs[0], sheetIndex, tracker);
 	}
+	
+	/**
+	 * Used by the lazy ref evals whenever they need to get the value of a contained cell.
+	 */
+	/* package */ ValueEval evaluateReference(HSSFSheet sheet, int sheetIndex, int rowIndex,
+			int columnIndex, EvaluationTracker tracker) {
+		
+		HSSFRow row = sheet.getRow(rowIndex);
+		HSSFCell cell;
+		if (row == null) {
+			cell = null;
+		} else {
+			cell = row.getCell(columnIndex);
+ 		}
+		CellLocation cellLoc = new CellLocation(sheetIndex, rowIndex, columnIndex);
+		tracker.acceptDependency(cellLoc);
+		return internalEvaluate(cell, cellLoc, tracker);
+	}
 }

Modified: poi/trunk/src/testcases/org/apache/poi/hssf/HSSFTests.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/testcases/org/apache/poi/hssf/HSSFTests.java?rev=698047&r1=698046&r2=698047&view=diff
==============================================================================
--- poi/trunk/src/testcases/org/apache/poi/hssf/HSSFTests.java (original)
+++ poi/trunk/src/testcases/org/apache/poi/hssf/HSSFTests.java Mon Sep 22 17:40:22 2008
@@ -28,6 +28,7 @@
 import org.apache.poi.hssf.record.AllRecordTests;
 import org.apache.poi.hssf.usermodel.AllUserModelTests;
 import org.apache.poi.hssf.util.AllHSSFUtilTests;
+import org.apache.poi.ss.formula.TestEvaluationCache;
 
 /**
  * Test Suite for all sub-packages of org.apache.poi.hssf<br/>
@@ -52,6 +53,7 @@
         }
         suite.addTest(new TestSuite(TestEventRecordFactory.class));
         suite.addTest(new TestSuite(TestModelFactory.class));
+        suite.addTest(new TestSuite(TestEvaluationCache.class));
         return suite;
     }
 }

Modified: poi/trunk/src/testcases/org/apache/poi/hssf/record/formula/eval/TestCircularReferences.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/testcases/org/apache/poi/hssf/record/formula/eval/TestCircularReferences.java?rev=698047&r1=698046&r2=698047&view=diff
==============================================================================
--- poi/trunk/src/testcases/org/apache/poi/hssf/record/formula/eval/TestCircularReferences.java (original)
+++ poi/trunk/src/testcases/org/apache/poi/hssf/record/formula/eval/TestCircularReferences.java Mon Sep 22 17:40:22 2008
@@ -71,8 +71,9 @@
 		// This formula should evaluate to the contents of B2,
 		testCell.setCellFormula("INDEX(A1:B4,2,2)");
 		// However the range A1:B4 also includes the current cell A4.  If the other parameters
-		// were 4 and 1, this would represent a circular reference.  Since POI 'fully' evaluates
-		// arguments before invoking operators, POI must handle such potential cycles gracefully.
+		// were 4 and 1, this would represent a circular reference.  Prior to v3.2 POI would 
+		// 'fully' evaluate ref arguments before invoking operators, which raised the possibility of
+		// cycles / StackOverflowErrors.
 		
 
 		CellValue cellValue = evaluateWithCycles(wb, testCell);

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=698047&r1=698046&r2=698047&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 Mon Sep 22 17:40:22 2008
@@ -31,6 +31,10 @@
 import org.apache.poi.hssf.record.formula.AreaPtg;
 import org.apache.poi.hssf.record.formula.FuncVarPtg;
 import org.apache.poi.hssf.record.formula.Ptg;
+import org.apache.poi.hssf.record.formula.eval.ValueEval;
+import org.apache.poi.ss.formula.EvaluationListener;
+import org.apache.poi.ss.formula.WorkbookEvaluator;
+import org.apache.poi.ss.formula.WorkbookEvaluatorTestHelper;
 
 /**
  * 
@@ -287,6 +291,29 @@
 		}
 	}
 	
+	private static final class EvalListener extends EvaluationListener {
+		private int _countCacheHits;
+		private int _countCacheMisses;
+
+		public EvalListener() {
+			_countCacheHits = 0;
+			_countCacheMisses = 0;
+		}
+		public int getCountCacheHits() {
+			return _countCacheHits;
+		}
+		public int getCountCacheMisses() {
+			return _countCacheMisses;
+		}
+
+		public void onCacheHit(int sheetIndex, int srcRowNum, int srcColNum, ValueEval result) {
+			_countCacheHits++;
+		}
+		public void onStartEvaluate(int sheetIndex, int rowIndex, int columnIndex, Ptg[] ptgs) {
+			_countCacheMisses++;
+		}
+	}
+	
 	/**
 	 * The HSSFFormula evaluator performance benefits greatly from caching of intermediate cell values
 	 */
@@ -312,16 +339,20 @@
 		
 		// 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(wb);
+		EvalListener evalListener = new EvalListener();
+		WorkbookEvaluator evaluator = WorkbookEvaluatorTestHelper.createEvaluator(wb, evalListener);
 		evaluator.evaluate(cell);
-		int evalCount = evaluator.getEvaluationCount();
-		// With caching, the evaluationCount is 8 which is a big improvement
-		assertTrue(evalCount > 0);  // make sure the counter is actually working
+		int evalCount = evalListener.getCountCacheMisses();
 		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");
 		}
+		// With caching, the evaluationCount is 8 which is a big improvement
+		// Note - these expected values may change if the WorkbookEvaluator is 
+		// ever optimised to short circuit 'if' functions.
+		assertEquals(8, evalCount);
+		assertEquals(24, evalListener.getCountCacheHits());
 	}
 }
\ No newline at end of file

Modified: poi/trunk/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFFormulaEvaluator.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFFormulaEvaluator.java?rev=698047&r1=698046&r2=698047&view=diff
==============================================================================
--- poi/trunk/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFFormulaEvaluator.java (original)
+++ poi/trunk/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFFormulaEvaluator.java Mon Sep 22 17:40:22 2008
@@ -17,10 +17,10 @@
 
 package org.apache.poi.hssf.usermodel;
 
+import junit.framework.TestCase;
+
 import org.apache.poi.hssf.HSSFTestDataSamples;
 import org.apache.poi.hssf.usermodel.HSSFFormulaEvaluator.CellValue;
-
-import junit.framework.TestCase;
 /**
  * 
  * @author Josh Micich
@@ -79,4 +79,17 @@
 		}
 		row.createCell(colIndex).setCellValue(value);
 	}
+
+	/**
+	 * {@link HSSFFormulaEvaluator#evaluate(HSSFCell)} should behave the same whether the cell
+	 * is <code>null</code> or blank.
+	 */
+	public void testEvaluateBlank() {
+		HSSFWorkbook wb = new HSSFWorkbook();
+		HSSFFormulaEvaluator fe = new HSSFFormulaEvaluator(wb);
+		assertNull(fe.evaluate(null));
+		HSSFSheet sheet = wb.createSheet("Sheet1");
+		HSSFCell cell = sheet.createRow(0).createCell(0);
+		assertNull(fe.evaluate(cell));
+	}
 }

Added: poi/trunk/src/testcases/org/apache/poi/ss/formula/EvaluationListener.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/testcases/org/apache/poi/ss/formula/EvaluationListener.java?rev=698047&view=auto
==============================================================================
--- poi/trunk/src/testcases/org/apache/poi/ss/formula/EvaluationListener.java (added)
+++ poi/trunk/src/testcases/org/apache/poi/ss/formula/EvaluationListener.java Mon Sep 22 17:40:22 2008
@@ -0,0 +1,53 @@
+/* ====================================================================
+   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.ss.formula;
+
+import org.apache.poi.hssf.record.formula.Ptg;
+import org.apache.poi.hssf.record.formula.eval.ValueEval;
+
+/**
+ * Tests should extend this class if they need to track the internal working of the {@link WorkbookEvaluator}.<br/>
+ * 
+ * Default method implementations all do nothing
+ * 
+ * @author Josh Micich
+ */
+public abstract class EvaluationListener implements IEvaluationListener {
+	public void onCacheHit(int sheetIndex, int rowIndex, int srcColNum, ValueEval result) {
+		// do nothing 
+	}
+	public void onReadPlainValue(int sheetIndex, int srcRowNum, int srcColNum, ValueEval value) {
+		// do nothing 
+	}
+	public void onStartEvaluate(int sheetIndex, int rowIndex, int columnIndex, Ptg[] ptgs) {
+		// do nothing 
+	}
+	public void onEndEvaluate(int sheetIndex, int rowIndex, int columnIndex, ValueEval result) {
+		// do nothing 
+	}
+	public void onClearWholeCache() {
+		// do nothing 
+	}
+	public void onClearCachedValue(int sheetIndex, int rowIndex, int columnIndex, ValueEval value) {
+		// do nothing 
+	}
+	public void onClearDependentCachedValue(int sheetIndex, int rowIndex, int columnIndex,
+			ValueEval value,int depth) {
+		// do nothing 
+	}
+}

Added: poi/trunk/src/testcases/org/apache/poi/ss/formula/TestEvaluationCache.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/testcases/org/apache/poi/ss/formula/TestEvaluationCache.java?rev=698047&view=auto
==============================================================================
--- poi/trunk/src/testcases/org/apache/poi/ss/formula/TestEvaluationCache.java (added)
+++ poi/trunk/src/testcases/org/apache/poi/ss/formula/TestEvaluationCache.java Mon Sep 22 17:40:22 2008
@@ -0,0 +1,492 @@
+/* ====================================================================
+   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.ss.formula;
+
+import java.io.PrintStream;
+import java.util.ArrayList;
+import java.util.List;
+
+import junit.framework.AssertionFailedError;
+import junit.framework.TestCase;
+
+import org.apache.poi.hssf.model.HSSFFormulaParser;
+import org.apache.poi.hssf.record.formula.Ptg;
+import org.apache.poi.hssf.record.formula.eval.BlankEval;
+import org.apache.poi.hssf.record.formula.eval.BoolEval;
+import org.apache.poi.hssf.record.formula.eval.ErrorEval;
+import org.apache.poi.hssf.record.formula.eval.NumberEval;
+import org.apache.poi.hssf.record.formula.eval.StringEval;
+import org.apache.poi.hssf.record.formula.eval.ValueEval;
+import org.apache.poi.hssf.usermodel.HSSFCell;
+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;
+
+/**
+ * Tests {@link EvaluationCache}.  Makes sure that where possible (previously calculated) cached 
+ * values are used.  Also checks that changing cell values causes the correct (minimal) set of
+ * dependent cached values to be cleared.
+ *
+ * @author Josh Micich
+ */
+public class TestEvaluationCache extends TestCase {
+	private static final class EvalListener extends EvaluationListener {
+
+		private final List _logList;
+		private final HSSFWorkbook _book;
+
+		public EvalListener(HSSFWorkbook wb) {
+			_book = wb;
+			_logList = new ArrayList();
+		}
+		public void onCacheHit(int sheetIndex, int rowIndex, int columnIndex, ValueEval result) {
+			log("hit", rowIndex, columnIndex, result);
+		}
+		public void onReadPlainValue(int sheetIndex, int rowIndex, int columnIndex, ValueEval value) {
+			log("value", rowIndex, columnIndex, value);
+		}
+		public void onStartEvaluate(int sheetIndex, int rowIndex, int columnIndex, Ptg[] ptgs) {
+			log("start", rowIndex, columnIndex, ptgs);
+		}
+		public void onEndEvaluate(int sheetIndex, int rowIndex, int columnIndex, ValueEval result) {
+			log("end", rowIndex, columnIndex, result);
+		}
+		public void onClearCachedValue(int sheetIndex, int rowIndex, int columnIndex, ValueEval value) {
+			log("clear", rowIndex, columnIndex, value);
+		}
+		public void onClearDependentCachedValue(int sheetIndex, int rowIndex, int columnIndex,
+				ValueEval value,int depth) {
+			log("clear" + depth, rowIndex, columnIndex, value);
+		}
+		private void log(String tag, int rowIndex, int columnIndex, Object value) {
+			StringBuffer sb = new StringBuffer(64);
+			sb.append(tag).append(' ');
+			sb.append(new CellReference(rowIndex, columnIndex).formatAsString());
+			if (value != null) {
+				sb.append(' ').append(formatValue(value));
+			}
+			_logList.add(sb.toString());
+		}
+		private String formatValue(Object value) {
+			if (value instanceof Ptg[]) {
+				Ptg[] ptgs = (Ptg[]) value;
+				return HSSFFormulaParser.toFormulaString(_book, ptgs);
+			}
+			if (value instanceof NumberEval) {
+				NumberEval ne = (NumberEval) value;
+				return ne.getStringValue();
+			}
+			if (value instanceof StringEval) {
+				StringEval se = (StringEval) value;
+				return "'" + se.getStringValue() + "'";
+			}
+			if (value instanceof BoolEval) {
+				BoolEval be = (BoolEval) value;
+				return be.getStringValue();
+			}
+			if (value == BlankEval.INSTANCE) {
+				return "#BLANK#";
+			}
+			if (value instanceof ErrorEval) {
+				ErrorEval ee = (ErrorEval) value;
+				return ErrorEval.getText(ee.getErrorCode());
+			}
+			throw new IllegalArgumentException("Unexpected value class ("
+					+ value.getClass().getName() + ")");
+		}
+		public String[] getAndClearLog() {
+			String[] result = new String[_logList.size()];
+			_logList.toArray(result);
+			_logList.clear();
+			return result;
+		}
+	}
+	/**
+	 * Wrapper class to manage repetitive tasks from this test,
+	 *
+	 * Note - this class does a little bit more than just plain set-up of data. The method
+	 * {@link WorkbookEvaluator#clearCachedResultValue(HSSFSheet, int, int)} is called whenever a
+	 * cell value is changed.
+	 *
+	 */
+	private static final class MySheet {
+
+		private final HSSFSheet _sheet;
+		private final WorkbookEvaluator _evaluator;
+		private final HSSFWorkbook _wb;
+		private final EvalListener _evalListener;
+
+		public MySheet() {
+			_wb = new HSSFWorkbook();
+			_evalListener = new EvalListener(_wb);
+			_evaluator = WorkbookEvaluatorTestHelper.createEvaluator(_wb, _evalListener);
+			_sheet = _wb.createSheet("Sheet1");
+		}
+
+		public void setCellValue(String cellRefText, double value) {
+			HSSFCell cell = getOrCreateCell(cellRefText);
+			// be sure to blank cell, in case it is currently a formula
+			cell.setCellType(HSSFCell.CELL_TYPE_BLANK);
+			// otherwise this line will only set the formula cached result;
+			cell.setCellValue(value);
+			_evaluator.setCachedPlainValue(_sheet, cell.getRowIndex(), cell.getCellNum(), new NumberEval(value));
+		}
+
+		public void setCellFormula(String cellRefText, String formulaText) {
+			HSSFCell cell = getOrCreateCell(cellRefText);
+			cell.setCellFormula(formulaText);
+			_evaluator.notifySetFormula(_sheet, cell.getRowIndex(), cell.getCellNum());
+		}
+
+		private HSSFCell getOrCreateCell(String cellRefText) {
+			CellReference cr = new CellReference(cellRefText);
+			int rowIndex = cr.getRow();
+			HSSFRow row = _sheet.getRow(rowIndex);
+			if (row == null) {
+				row = _sheet.createRow(rowIndex);
+			}
+			int cellIndex = cr.getCol();
+			HSSFCell cell = row.getCell(cellIndex);
+			if (cell == null) {
+				cell = row.createCell(cellIndex);
+			}
+			return cell;
+		}
+
+		public ValueEval evaluateCell(String cellRefText) {
+			return _evaluator.evaluate(getOrCreateCell(cellRefText));
+		}
+
+		public String[] getAndClearLog() {
+			return _evalListener.getAndClearLog();
+		}
+
+		public void clearAllCachedResultValues() {
+			_evaluator.clearAllCachedResultValues();
+		}
+	}
+
+	private static MySheet createMediumComplex() {
+		MySheet ms = new MySheet();
+
+		// plain data in D1:F3
+		ms.setCellValue("D1", 12);
+		ms.setCellValue("E1", 13);
+		ms.setCellValue("D2", 14);
+		ms.setCellValue("E2", 15);
+		ms.setCellValue("D3", 16);
+		ms.setCellValue("E3", 17);
+
+
+		ms.setCellFormula("C1", "SUM(D1:E2)");
+		ms.setCellFormula("C2", "SUM(D2:E3)");
+		ms.setCellFormula("C3", "SUM(D3:E4)");
+
+		ms.setCellFormula("B1", "C2-C1");
+		ms.setCellFormula("B2", "B3*C1-C2");
+		ms.setCellValue("B3", 2);
+
+		ms.setCellFormula("A1", "MAX(B1:B2)");
+		ms.setCellFormula("A2", "MIN(B3,D2:F2)");
+		ms.setCellFormula("A3", "B3*C3");
+
+		// clear all the logging from the above initialisation
+		ms.getAndClearLog();
+		ms.clearAllCachedResultValues();
+		return ms;
+	}
+
+	public void testMediumComplex() {
+
+		MySheet ms = createMediumComplex();
+		// completely fresh evaluation
+		confirmEvaluate(ms, "A1", 46);
+		confirmLog(ms, new String[] {
+			"start A1 MAX(B1:B2)",
+				"start B1 C2-C1",
+					"start C2 SUM(D2:E3)",
+						"value D2 14", "value E2 15", "value D3 16", "value E3 17",
+					"end C2 62",
+					"start C1 SUM(D1:E2)",
+						"value D1 12", "value E1 13", "hit D2 14", "hit E2 15",
+					"end C1 54",
+				"end B1 8",
+				"start B2 B3*C1-C2",
+					"value B3 2",
+					"hit C1 54",
+					"hit C2 62",
+				"end B2 46",
+			"end A1 46",
+		});
+
+
+		// simple cache hit - immediate re-evaluation with no changes
+		confirmEvaluate(ms, "A1", 46);
+		confirmLog(ms, new String[] { "hit A1 46", });
+
+		// change a low level cell
+		ms.setCellValue("D1", 10);
+		confirmLog(ms, new String[] {
+				"clear1 C1 54",
+				"clear2 B1 8",
+				"clear3 A1 46",
+				"clear2 B2 46",
+		});
+		confirmEvaluate(ms, "A1", 42);
+		confirmLog(ms, new String[] {
+			"start A1 MAX(B1:B2)",
+				"start B1 C2-C1",
+					"hit C2 62",
+					"start C1 SUM(D1:E2)",
+						"hit D1 10", "hit E1 13", "hit D2 14", "hit E2 15",
+					"end C1 52",
+				"end B1 10",
+				"start B2 B3*C1-C2",
+					"hit B3 2",
+					"hit C1 52",
+					"hit C2 62",
+				"end B2 42",
+			"end A1 42",
+		});
+
+		// Reset and try changing an intermediate value
+		ms = createMediumComplex();
+		confirmEvaluate(ms, "A1", 46);
+		ms.getAndClearLog();
+
+		ms.setCellValue("B3", 3); // B3 is in the middle of the dependency tree
+		confirmLog(ms, new String[] {
+				"clear1 B2 46",
+				"clear2 A1 46",
+		});
+		confirmEvaluate(ms, "A1", 100);
+		confirmLog(ms, new String[] {
+			"start A1 MAX(B1:B2)",
+				"hit B1 8",
+				"start B2 B3*C1-C2",
+					"hit B3 3",
+					"hit C1 54",
+					"hit C2 62",
+				"end B2 100",
+			"end A1 100",
+		});
+	}
+
+	public void testMediumComplexWithDependencyChange() {
+
+		// Changing an intermediate formula
+		MySheet ms = createMediumComplex();
+		confirmEvaluate(ms, "A1", 46);
+		ms.getAndClearLog();
+		ms.setCellFormula("B2", "B3*C2-C3"); // used to be "B3*C1-C2"
+		confirmLog(ms, new String[] {
+			"clear1 A1 46",
+		});
+
+		confirmEvaluate(ms, "A1", 91);
+		confirmLog(ms, new String[] {
+			"start A1 MAX(B1:B2)",
+				"hit B1 8",
+				"start B2 B3*C2-C3",
+					"hit B3 2",
+					"hit C2 62",
+					"start C3 SUM(D3:E4)",
+						"hit D3 16", "hit E3 17", "value D4 #BLANK#", "value E4 #BLANK#",
+					"end C3 33",
+				"end B2 91",
+			"end A1 91",
+		});
+
+		//----------------
+		// Note - From now on the demonstrated POI behaviour is not optimal
+		//----------------
+
+		// Now change a value that should no longer affect B2
+		ms.setCellValue("D1", 11);
+		confirmLog(ms, new String[] {
+			"clear1 C1 54",
+			// note there is no "clear2 B2 91" here because B2 doesn't depend on C1 anymore
+			"clear2 B1 8",
+			"clear3 A1 91",
+		});
+
+		confirmEvaluate(ms, "B2", 91);
+		confirmLog(ms, new String[] {
+			"hit B2 91",  // further confirmation that B2 was not cleared due to changing D1 above
+		});
+
+		// things should be back to normal now
+		ms.setCellValue("D1", 11);
+		confirmLog(ms, new String[] {  });
+		confirmEvaluate(ms, "B2", 91);
+		confirmLog(ms, new String[] {
+			"hit B2 91",
+		});
+	}
+
+	/**
+	 * verifies that when updating a plain cell, depending (formula) cell cached values are cleared
+	 * only when the palin cell's value actually changes
+	 */
+	public void testRedundantUpdate() {
+		MySheet ms = new MySheet();
+
+		ms.setCellValue("B1", 12);
+		ms.setCellValue("C1", 13);
+		ms.setCellFormula("A1", "B1+C1");
+
+		// evaluate twice to confirm caching looks OK
+		ms.evaluateCell("A1");
+		ms.getAndClearLog();
+		confirmEvaluate(ms, "A1", 25);
+		confirmLog(ms, new String[] {
+			"hit A1 25",
+		});
+
+		// Make redundant update, and check re-evaluation
+		ms.setCellValue("B1", 12); // value didn't change
+		confirmLog(ms, new String[] {});
+		confirmEvaluate(ms, "A1", 25);
+		confirmLog(ms, new String[] {
+			"hit A1 25",
+		});
+
+		ms.setCellValue("B1", 11); // value changing
+		confirmLog(ms, new String[] {
+			"clear1 A1 25",	// expect consuming formula cached result to get cleared
+		});
+		confirmEvaluate(ms, "A1", 24);
+		confirmLog(ms, new String[] {
+			"start A1 B1+C1",
+			"hit B1 11",
+			"hit C1 13",
+			"end A1 24",
+		});
+	}
+
+	/**
+	 * Changing any input to a formula may cause the formula to 'use' a different set of cells.
+	 * Functions like INDEX and OFFSET make this effect obvious, with functions like MATCH
+	 * and VLOOKUP the effect can be subtle.  The presence of error values can also produce this
+	 * effect in almost every function and operator.
+	 */
+	public void testSimpleWithDependencyChange() {
+
+		MySheet ms = new MySheet();
+
+		ms.setCellFormula("A1", "INDEX(C1:E1,1,B1)");
+		ms.setCellValue("B1", 1);
+		ms.setCellValue("C1", 17);
+		ms.setCellValue("D1", 18);
+		ms.setCellValue("E1", 19);
+		ms.clearAllCachedResultValues();
+		ms.getAndClearLog();
+
+		confirmEvaluate(ms, "A1", 17);
+		confirmLog(ms, new String[] {
+			"start A1 INDEX(C1:E1,1,B1)",
+			"value B1 1",
+			"value C1 17",
+			"end A1 17",
+		});
+		ms.setCellValue("B1", 2);
+		ms.getAndClearLog();
+
+		confirmEvaluate(ms, "A1", 18);
+		confirmLog(ms, new String[] {
+			"start A1 INDEX(C1:E1,1,B1)",
+			"hit B1 2",
+			"value D1 18",
+			"end A1 18",
+		});
+
+		// change C1. Note - last time A1 evaluated C1 was not used
+		ms.setCellValue("C1", 15);
+		ms.getAndClearLog();
+		confirmEvaluate(ms, "A1", 18);
+		confirmLog(ms, new String[] {
+			"hit A1 18",
+		});
+
+		// but A1 still uses D1, so if it changes...
+		ms.setCellValue("D1", 25);
+		ms.getAndClearLog();
+		confirmEvaluate(ms, "A1", 25);
+		confirmLog(ms, new String[] {
+			"start A1 INDEX(C1:E1,1,B1)",
+			"hit B1 2",
+			"hit D1 25",
+			"end A1 25",
+		});
+
+	}
+
+	private static void confirmEvaluate(MySheet ms, String cellRefText, double expectedValue) {
+		ValueEval v = ms.evaluateCell(cellRefText);
+		assertEquals(NumberEval.class, v.getClass());
+		assertEquals(expectedValue, ((NumberEval)v).getNumberValue(), 0.0);
+	}
+
+	private static void confirmLog(MySheet ms, String[] expectedLog) {
+		String[] actualLog = ms.getAndClearLog();
+		int endIx = actualLog.length;
+		PrintStream ps = System.err;
+		if (endIx != expectedLog.length) {
+			ps.println("Log lengths mismatch");
+			dumpCompare(ps, expectedLog, actualLog);
+			throw new AssertionFailedError("Log lengths mismatch");
+		}
+		for (int i=0; i< endIx; i++) {
+			if (!actualLog[i].equals(expectedLog[i])) {
+				String msg = "Log entry mismatch at index " + i;
+				ps.println(msg);
+				dumpCompare(ps, expectedLog, actualLog);
+				throw new AssertionFailedError(msg);
+			}
+		}
+
+	}
+
+	private static void dumpCompare(PrintStream ps, String[] expectedLog, String[] actualLog) {
+		int max = Math.max(actualLog.length, expectedLog.length);
+		ps.println("Index\tExpected\tActual");
+		for(int i=0; i<max; i++) {
+			ps.print(i + "\t");
+			printItem(ps, expectedLog, i);
+			ps.print("\t");
+			printItem(ps, actualLog, i);
+			ps.println();
+		}
+		ps.println();
+		debugPrint(ps, actualLog);
+	}
+
+	private static void printItem(PrintStream ps, String[] ss, int index) {
+		if (index < ss.length) {
+			ps.print(ss[index]);
+		}
+	}
+
+	private static void debugPrint(PrintStream ps, String[] log) {
+		for (int i = 0; i < log.length; i++) {
+			ps.println('"' + log[i] + "\",");
+
+		}
+	}
+}

Added: poi/trunk/src/testcases/org/apache/poi/ss/formula/WorkbookEvaluatorTestHelper.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/testcases/org/apache/poi/ss/formula/WorkbookEvaluatorTestHelper.java?rev=698047&view=auto
==============================================================================
--- poi/trunk/src/testcases/org/apache/poi/ss/formula/WorkbookEvaluatorTestHelper.java (added)
+++ poi/trunk/src/testcases/org/apache/poi/ss/formula/WorkbookEvaluatorTestHelper.java Mon Sep 22 17:40:22 2008
@@ -0,0 +1,37 @@
+/* ====================================================================
+   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.ss.formula;
+
+import org.apache.poi.hssf.usermodel.HSSFEvaluationWorkbook;
+import org.apache.poi.hssf.usermodel.HSSFWorkbook;
+
+/**
+ * Allows tests to execute {@link WorkbookEvaluator}s and track the internal workings.
+ *  
+ * @author Josh Micich
+ */
+public final class WorkbookEvaluatorTestHelper {
+
+	private WorkbookEvaluatorTestHelper() {
+		// no instances of this class
+	}
+	
+	public static WorkbookEvaluator createEvaluator(HSSFWorkbook wb, EvaluationListener listener) {
+		return new WorkbookEvaluator(HSSFEvaluationWorkbook.create(wb), listener);
+	}
+}



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