Bug 51171: Improved performance of SharedValueManager

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1127860 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/src/documentation/content/xdocs/status.xml b/src/documentation/content/xdocs/status.xml
index cd9a6ede285ac8ca525f781741b794e9281a9df4..c0790c9f337b306c4b96b23b64a361f4663e1ef3 100644 (file)
--- a/src/documentation/content/xdocs/status.xml
+++ b/src/documentation/content/xdocs/status.xml
@@ -34,6 +34,7 @@
 
     <changes>
         <release version="3.8-beta3" date="2011-??-??">
+           <action dev="poi-developers" type="add">51171 - Improved performance of SharedValueManager </action>
            <action dev="poi-developers" type="fix">51236 - XSSF set colour support for black/white to match getter</action>
            <action dev="poi-developers" type="add">51196 - Initial support for Spreadsheet Chart API</action>
            <action dev="poi-developers" type="add">Add support for OOXML Agile Encryption</action>

diff --git a/src/java/org/apache/poi/hssf/record/aggregates/SharedValueManager.java b/src/java/org/apache/poi/hssf/record/aggregates/SharedValueManager.java
index c393d44e7b56df623088b9fc60cd7c61d4da871e..81abdb6fd971864d530a0d4c171c958e207bf66f 100644 (file)
--- a/src/java/org/apache/poi/hssf/record/aggregates/SharedValueManager.java
+++ b/src/java/org/apache/poi/hssf/record/aggregates/SharedValueManager.java
@@ -18,8 +18,6 @@
 package org.apache.poi.hssf.record.aggregates;
 
 import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Comparator;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -74,7 +72,7 @@ public final class SharedValueManager {
                public void add(FormulaRecordAggregate agg) {
                        if (_numberOfFormulas == 0) {
                                if (_firstCell.getRow() != agg.getRow() || _firstCell.getCol() != agg.getColumn()) {
-                                       throw new IllegalStateException("shared formula coding error");
+                                       throw new IllegalStateException("shared formula coding error: "+_firstCell.getCol()+'/'+_firstCell.getRow()+" != "+agg.getColumn()+'/'+agg.getRow());
                                }
                        }
                        if (_numberOfFormulas >= _frAggs.length) {
@@ -100,16 +98,6 @@ public final class SharedValueManager {
                        sb.append("]");
                        return sb.toString();
                }
-
-               /**
-                * Note - the 'first cell' of a shared formula group is not always the top-left cell
-                * of the enclosing range.
-                * @return <code>true</code> if the specified coordinates correspond to the 'first cell'
-                * of this shared formula group.
-                */
-               public boolean isFirstCell(int row, int column) {
-                       return _firstCell.getRow() == row && _firstCell.getCol() == column;
-               }
        }
 
        /**
@@ -124,7 +112,7 @@ public final class SharedValueManager {
        private final TableRecord[] _tableRecords;
        private final Map<SharedFormulaRecord, SharedFormulaGroup> _groupsBySharedFormulaRecord;
        /** cached for optimization purposes */
-       private SharedFormulaGroup[] _groups;
+    private Map<Integer,SharedFormulaGroup> _groupsCache;
 
        private SharedValueManager(SharedFormulaRecord[] sharedFormulaRecords,
                        CellReference[] firstCells, ArrayRecord[] arrayRecords, TableRecord[] tableRecords) {
@@ -169,56 +157,30 @@ public final class SharedValueManager {
         * @return never <code>null</code>
         */
        public SharedFormulaRecord linkSharedFormulaRecord(CellReference firstCell, FormulaRecordAggregate agg) {
-
-               SharedFormulaGroup result = findFormulaGroup(getGroups(), firstCell);
+               SharedFormulaGroup result = findFormulaGroupForCell(firstCell);
                result.add(agg);
                return result.getSFR();
        }
 
-       private static SharedFormulaGroup findFormulaGroup(SharedFormulaGroup[] groups, CellReference firstCell) {
-               int row = firstCell.getRow();
-               int column = firstCell.getCol();
-               // Traverse the list of shared formulas and try to find the correct one for us
-
-               // perhaps this could be optimised to some kind of binary search
-               for (int i = 0; i < groups.length; i++) {
-                       SharedFormulaGroup svg = groups[i];
-                       if (svg.isFirstCell(row, column)) {
-                               return svg;
-                       }
-               }
-               // TODO - fix file "15228.xls" so it opens in Excel after rewriting with POI
-               throw new RuntimeException("Failed to find a matching shared formula record");
-       }
-
-       private SharedFormulaGroup[] getGroups() {
-               if (_groups == null) {
-                       SharedFormulaGroup[] groups = new SharedFormulaGroup[_groupsBySharedFormulaRecord.size()];
-                       _groupsBySharedFormulaRecord.values().toArray(groups);
-                       Arrays.sort(groups, SVGComparator); // make search behaviour more deterministic
-                       _groups = groups;
-               }
-               return _groups;
-       }
-
-       private static final Comparator<SharedFormulaGroup> SVGComparator = new Comparator<SharedFormulaGroup>() {
-
-               public int compare(SharedFormulaGroup a, SharedFormulaGroup b) {
-                       CellRangeAddress8Bit rangeA = a.getSFR().getRange();
-                       CellRangeAddress8Bit rangeB = b.getSFR().getRange();
-
-                       int cmp;
-                       cmp = rangeA.getFirstRow() - rangeB.getFirstRow();
-                       if (cmp != 0) {
-                               return cmp;
-                       }
-                       cmp = rangeA.getFirstColumn() - rangeB.getFirstColumn();
-                       if (cmp != 0) {
-                               return cmp;
-                       }
-                       return 0;
-               }
-       };
+    private SharedFormulaGroup findFormulaGroupForCell(final CellReference cellRef) {
+        if(null == _groupsCache) {
+            _groupsCache = new HashMap<Integer,SharedFormulaGroup>(_groupsBySharedFormulaRecord.size());
+            for(SharedFormulaGroup group: _groupsBySharedFormulaRecord.values()) {
+                _groupsCache.put(getKeyForCache(group._firstCell),group);
+            }
+        }
+        SharedFormulaGroup sfg = _groupsCache.get(getKeyForCache(cellRef));
+        if(null == sfg) {
+            // TODO - fix file "15228.xls" so it opens in Excel after rewriting with POI
+            throw new RuntimeException("Failed to find a matching shared formula record");
+        }
+        return sfg;
+    }
+
+    private Integer getKeyForCache(final CellReference cellRef) {
+        // The HSSF has a max of 2^16 rows and 2^8 cols
+        return new Integer((cellRef.getCol()+1)<<16 | cellRef.getRow());
+    }
 
        /**
         * Gets the {@link SharedValueRecordBase} record if it should be encoded immediately after the
@@ -248,15 +210,13 @@ public final class SharedValueManager {
                        // not the first formula cell in the group
                        return null;
                }
-               SharedFormulaGroup[] groups = getGroups();
-               for (int i = 0; i < groups.length; i++) {
-                       // note - logic for finding correct shared formula group is slightly
-                       // more complicated since the first cell
-                       SharedFormulaGroup sfg = groups[i];
-                       if (sfg.isFirstCell(row, column)) {
-                               return sfg.getSFR();
-                       }
-               }
+
+        if(!_groupsBySharedFormulaRecord.isEmpty()) {
+            SharedFormulaGroup sfg = findFormulaGroupForCell(firstCell);
+            if(null != sfg) {
+                return sfg.getSFR();
+            }
+        }
 
                // Since arrays and tables cannot be sparse (all cells in range participate)
                // The first cell will be the top left in the range.  So we can match the
@@ -284,7 +244,7 @@ public final class SharedValueManager {
                if (svg == null) {
                        throw new IllegalStateException("Failed to find formulas for shared formula");
                }
-               _groups = null; // be sure to reset cached value
+               _groupsCache = null; // be sure to reset cached value
                svg.unlinkSharedFormulas();
        }