Replace Hashtable/ArrayList with generic collections in formula eval hot path

benchmarks/NPOI.Benchmarks/Program.cs

@@ -1,4 +1,4 @@
 using BenchmarkDotNet.Running;
 using NPOI.Benchmarks;
 
-BenchmarkSwitcher.FromAssembly(typeof(LargeExcelFileBenchmark).Assembly).Run();
+BenchmarkSwitcher.FromAssembly(typeof(LargeExcelFileBenchmark).Assembly).Run(args);

main/SS/Formula/Eval/TwoOperandNumeric/MultiplyEval.cs

@@ -25,10 +25,18 @@ public class MultiplyEval : TwoOperandNumericOperation
     {
         public override double Evaluate(double d0, double d1)
         {
-            decimal dec0 = (decimal)d0;
-            decimal dec1 = (decimal)d1;
+            try
+            {
+                decimal dec0 = (decimal)d0;
+                decimal dec1 = (decimal)d1;
 
-            return decimal.ToDouble(dec0 * dec1);
+                return decimal.ToDouble(dec0 * dec1);
+            }
+            catch (System.OverflowException)
+            {
+                // Values too large for decimal — fall back to double arithmetic (matches Excel behavior)
+                return d0 * d1;
+            }
         }
     }
 }

main/SS/Formula/FormulaCellCache.cs

@@ -17,7 +17,7 @@ limitations under the License.
 
 namespace NPOI.SS.Formula
 {
-    using System.Collections;
+    using System.Collections.Generic;
 
     public interface IEntryOperation
     {
@@ -31,19 +31,18 @@ public interface IEntryOperation
     public class FormulaCellCache
     {
 
-        private readonly Hashtable _formulaEntriesByCell;
+        private readonly Dictionary<object, FormulaCellCacheEntry> _formulaEntriesByCell;
 
         public FormulaCellCache()
         {
             // assumes HSSFCell does not override HashCode or Equals, otherwise we need IdentityHashMap
-            _formulaEntriesByCell = new Hashtable();
+            _formulaEntriesByCell = new Dictionary<object, FormulaCellCacheEntry>();
         }
 
         public CellCacheEntry[] GetCacheEntries()
         {
-
             FormulaCellCacheEntry[] result = new FormulaCellCacheEntry[_formulaEntriesByCell.Count];
-            _formulaEntriesByCell.Values.CopyTo(result,0);
+            _formulaEntriesByCell.Values.CopyTo(result, 0);
             return result;
         }
 
@@ -57,10 +56,8 @@ public void Clear()
          */
         public FormulaCellCacheEntry Get(IEvaluationCell cell)
         {
-            if (_formulaEntriesByCell.ContainsKey(cell.IdentityKey))
-                return (FormulaCellCacheEntry)_formulaEntriesByCell[cell.IdentityKey];
-            else
-                return null;
+            _formulaEntriesByCell.TryGetValue(cell.IdentityKey, out FormulaCellCacheEntry entry);
+            return entry;
         }
 
         public void Put(IEvaluationCell cell, FormulaCellCacheEntry entry)
@@ -70,17 +67,16 @@ public void Put(IEvaluationCell cell, FormulaCellCacheEntry entry)
 
         public FormulaCellCacheEntry Remove(IEvaluationCell cell)
         {
-            FormulaCellCacheEntry tmp = (FormulaCellCacheEntry)_formulaEntriesByCell[cell.IdentityKey];
-            _formulaEntriesByCell.Remove(cell);
+            _formulaEntriesByCell.TryGetValue(cell.IdentityKey, out FormulaCellCacheEntry tmp);
+            _formulaEntriesByCell.Remove(cell.IdentityKey);
             return tmp;
         }
 
         public void ApplyOperation(IEntryOperation operation)
         {
-            IEnumerator i = _formulaEntriesByCell.Values.GetEnumerator();
-            while (i.MoveNext())
+            foreach (FormulaCellCacheEntry entry in _formulaEntriesByCell.Values)
             {
-                operation.ProcessEntry((FormulaCellCacheEntry)i.Current);
+                operation.ProcessEntry(entry);
             }
         }
     }

main/SS/Formula/FormulaParser.cs

@@ -19,7 +19,6 @@ namespace NPOI.SS.Formula
 {
 
     using System;
-    using System.Collections;
     using System.Collections.Generic;
     using System.Globalization;
     using System.Text; 
@@ -1798,7 +1797,7 @@ private static bool IsArgumentDelimiter(int ch)
         private ParseNode[] Arguments()
         {
             //average 2 args per Function
-            ArrayList temp = new ArrayList(2);
+            List<ParseNode> temp = new List<ParseNode>(2);
             SkipWhite();
             if (look == ')')
             {
@@ -1834,7 +1833,7 @@ private ParseNode[] Arguments()
                     throw expected("',' or ')'");
                 }
             }
-            ParseNode[] result = (ParseNode[])temp.ToArray(typeof(ParseNode));
+            ParseNode[] result = temp.ToArray();
             return result;
         }
 
@@ -1989,7 +1988,7 @@ private static void CheckRowLengths(Object[][] values2d, int nColumns)
 
         private Object[] ParseArrayRow()
         {
-            ArrayList temp = new ArrayList();
+            List<object> temp = new List<object>();
             while (true)
             {
                 temp.Add(ParseArrayItem());
@@ -2009,9 +2008,7 @@ private Object[] ParseArrayRow()
                 break;
             }
 
-            Object[] result = new Object[temp.Count];
-            result = temp.ToArray();
-            return result;
+            return temp.ToArray();
         }
 
         private Object ParseArrayItem()

main/SS/Formula/FormulaUsedBlankCellSet.cs

@@ -15,14 +15,12 @@ the License.  You may obtain a copy of the License at
    limitations under the License.
 ==================================================================== */
 
-using System.Collections.Generic;
-
 namespace NPOI.SS.Formula
 {
 
     using System;
     using System.Text;
-    using System.Collections;
+    using System.Collections.Generic;
     using NPOI.SS.Util;
 
     public class BookSheetKey
@@ -202,11 +200,11 @@ public override String ToString()
             }
         }
 
-        private readonly Hashtable _sheetGroupsByBookSheet;
+        private readonly Dictionary<BookSheetKey, BlankCellSheetGroup> _sheetGroupsByBookSheet;
 
         public FormulaUsedBlankCellSet()
         {
-            _sheetGroupsByBookSheet = new Hashtable();
+            _sheetGroupsByBookSheet = new Dictionary<BookSheetKey, BlankCellSheetGroup>();
         }
 
         public void AddCell(IEvaluationWorkbook evalWorkbook, int bookIndex, int sheetIndex, int rowIndex, int columnIndex)
@@ -219,19 +217,17 @@ private BlankCellSheetGroup GetSheetGroup(IEvaluationWorkbook evalWorkbook, int
         {
             BookSheetKey key = new BookSheetKey(bookIndex, sheetIndex);
 
-            BlankCellSheetGroup result = (BlankCellSheetGroup)_sheetGroupsByBookSheet[key];
-            if (result == null)
+            if (!_sheetGroupsByBookSheet.TryGetValue(key, out BlankCellSheetGroup result))
             {
                 result = new BlankCellSheetGroup(evalWorkbook.GetSheet(sheetIndex).LastRowNum);
-                _sheetGroupsByBookSheet[key]= result;
+                _sheetGroupsByBookSheet[key] = result;
             }
             return result;
         }
 
         public bool ContainsCell(BookSheetKey key, int rowIndex, int columnIndex)
         {
-            BlankCellSheetGroup bcsg = (BlankCellSheetGroup)_sheetGroupsByBookSheet[key];
-            if (bcsg == null)
+            if (!_sheetGroupsByBookSheet.TryGetValue(key, out BlankCellSheetGroup bcsg))
             {
                 return false;
             }

main/SS/Formula/OperationEvaluatorFactory.cs

@@ -19,7 +19,7 @@ namespace NPOI.SS.Formula
 {
 
     using System;
-    using System.Collections;
+    using System.Collections.Generic;
     using System.Reflection;
 
     using NPOI.SS.Formula.Eval;
@@ -36,16 +36,16 @@ class OperationEvaluatorFactory
     {
         private static Type[] OPERATION_CONSTRUCTOR_CLASS_ARRAY = new Type[] { typeof(Ptg) };
 
-        private static readonly Hashtable _instancesByPtgClass = InitialiseInstancesMap();
+        private static readonly Dictionary<OperationPtg, Functions.Function> _instancesByPtgClass = InitialiseInstancesMap();
 
         private OperationEvaluatorFactory()
         {
             // no instances of this class
         }
 
-        private static Hashtable InitialiseInstancesMap()
+        private static Dictionary<OperationPtg, Functions.Function> InitialiseInstancesMap()
         {
-            Hashtable m = new Hashtable(32);
+            Dictionary<OperationPtg, Functions.Function> m = new Dictionary<OperationPtg, Functions.Function>(32);
             Add(m, EqualPtg.instance, RelationalOperationEval.EqualEval);
             Add(m, GreaterEqualPtg.instance, RelationalOperationEval.GreaterEqualEval);
             Add(m, GreaterThanPtg.instance, RelationalOperationEval.GreaterThanEval);
@@ -67,7 +67,7 @@ private static Hashtable InitialiseInstancesMap()
             return m;
         }
 
-        private static void Add(Hashtable m, OperationPtg ptgKey, Functions.Function instance)
+        private static void Add(Dictionary<OperationPtg, Functions.Function> m, OperationPtg ptgKey, Functions.Function instance)
         {
             // make sure ptg has single private constructor because map lookups assume singleton keys
             ConstructorInfo[] cc = ptgKey.GetType().GetConstructors();
@@ -91,7 +91,7 @@ public static ValueEval Evaluate(OperationPtg ptg, ValueEval[] args,
             {
                 throw new ArgumentException("ptg must not be null");
             }
-            NPOI.SS.Formula.Functions.Function result = _instancesByPtgClass[ptg] as NPOI.SS.Formula.Functions.Function;
+            _instancesByPtgClass.TryGetValue(ptg, out NPOI.SS.Formula.Functions.Function result);
 
             FreeRefFunction udfFunc = null;
             if (result == null)

main/SS/Formula/PTG/Ptg.cs

@@ -20,11 +20,9 @@ namespace NPOI.SS.Formula.PTG
 
     using System;
     using System.IO;
-    using System.Runtime.Serialization.Formatters.Binary;
-    using System.Collections;
+    using System.Collections.Generic;
 
     using NPOI.Util;
-    using System.Collections.Generic;
 
 
     /**
@@ -54,7 +52,7 @@ public abstract class Ptg : ICloneable
          */
         public static Ptg[] ReadTokens(int size, ILittleEndianInput in1)
         {
-            ArrayList temp = new ArrayList(4 + size / 2);
+            List<Ptg> temp = new List<Ptg>(4 + size / 2);
             int pos = 0;
             bool hasArrayPtgs = false;
             while (pos < size)
@@ -180,15 +178,14 @@ private static Ptg CreateBasePtg(byte id, ILittleEndianInput in1)
             }
             throw new Exception("Unexpected base token id (" + id + ")");
         }
-        private static Ptg[] ToPtgArray(ArrayList l)
+        private static Ptg[] ToPtgArray(List<Ptg> l)
         {
             if (l.Count == 0)
             {
                 return EMPTY_PTG_ARRAY;
             }
 
-            Ptg[] result = (Ptg[])l.ToArray(typeof(Ptg));
-            return result;
+            return l.ToArray();
         }
         /**
          * @return a distinct copy of this <c>Ptg</c> if the class is mutable, or the same instance

main/SS/Formula/PlainCellCache.cs

@@ -19,7 +19,7 @@ namespace NPOI.SS.Formula
 {
 
     using System;
-    using System.Collections;
+    using System.Collections.Generic;
     using NPOI.Util;
 
     public class Loc
@@ -93,11 +93,11 @@ public int BookIndex
     public class PlainCellCache
     {
 
-        private readonly Hashtable _plainValueEntriesByLoc;
+        private readonly Dictionary<Loc, PlainValueCellCacheEntry> _plainValueEntriesByLoc;
 
         public PlainCellCache()
         {
-            _plainValueEntriesByLoc = new Hashtable();
+            _plainValueEntriesByLoc = new Dictionary<Loc, PlainValueCellCacheEntry>();
         }
         public void Put(Loc key, PlainValueCellCacheEntry cce)
         {
@@ -109,7 +109,8 @@ public void Clear()
         }
         public PlainValueCellCacheEntry Get(Loc key)
         {
-            return (PlainValueCellCacheEntry)_plainValueEntriesByLoc[key];
+            _plainValueEntriesByLoc.TryGetValue(key, out PlainValueCellCacheEntry entry);
+            return entry;
         }
         public void Remove(Loc key)
         {

testcases/main/SS/Formula/Eval/TestMultiplyEval.cs

@@ -47,9 +47,23 @@ public void TestBasic()
             System.Threading.Thread.CurrentThread.CurrentCulture = System.Globalization.CultureInfo.CreateSpecificCulture("en-US");
 
             // issue #1055, use decimal to handle precision issue like (20000 * 0.000027 = 0.539999 in double)
-            Confirm(new NumberEval(20000), new NumberEval(0.000027), 0.54);            
+            Confirm(new NumberEval(20000), new NumberEval(0.000027), 0.54);
         }
-
+
+        [Test]
+        public void TestLargeValuesOverflowDecimal()
+        {
+            // Values > ~7.9e28 overflow decimal; should fall back to double arithmetic
+            // Use direct assertions with relative tolerance since Confirm uses delta=0
+            ValueEval[] args1 = { new NumberEval(1e29), new NumberEval(1e29) };
+            double result1 = NumericFunctionInvoker.Invoke(EvalInstances.Multiply, args1, 0, 0);
+            ClassicAssert.AreEqual(1e58, result1, 1e58 * 1e-10, "Large positive multiply");
+
+            ValueEval[] args2 = { new NumberEval(-1e30), new NumberEval(1e30) };
+            double result2 = NumericFunctionInvoker.Invoke(EvalInstances.Multiply, args2, 0, 0);
+            ClassicAssert.AreEqual(-1e60, result2, 1e60 * 1e-10, "Mixed sign large multiply");
+        }
+
     }
 
 }

testcases/main/SS/Formula/TestEvaluationCache.cs

@@ -847,6 +847,48 @@ public void TestPlainValueCache()
             ClassicAssert.AreEqual(8394753.0, summaryCell.NumericCellValue);
         }
 
+        /// <summary>
+        /// Verifies that FormulaCellCache.Remove() actually evicts entries.
+        /// Regression test for bug where Remove() used cell instead of cell.IdentityKey,
+        /// causing entries to never be removed from the cache.
+        /// </summary>
+        [Test]
+        public void TestFormulaCellCacheRemoveActuallyEvicts()
+        {
+            HSSFWorkbook wb = new HSSFWorkbook();
+            ISheet sheet = wb.CreateSheet();
+            IRow row = sheet.CreateRow(0);
+
+            ICell cellA1 = row.CreateCell(0);
+            cellA1.SetCellFormula("1+1");
+            ICell cellB1 = row.CreateCell(1);
+            cellB1.SetCellFormula("2+2");
+
+            IEvaluationCell evalCellA1 = HSSFEvaluationTestHelper.WrapCell(cellA1);
+            IEvaluationCell evalCellB1 = HSSFEvaluationTestHelper.WrapCell(cellB1);
+
+            FormulaCellCache cache = new FormulaCellCache();
+            FormulaCellCacheEntry entryA1 = new FormulaCellCacheEntry();
+            FormulaCellCacheEntry entryB1 = new FormulaCellCacheEntry();
+
+            cache.Put(evalCellA1, entryA1);
+            cache.Put(evalCellB1, entryB1);
+
+            // Both entries should be retrievable
+            ClassicAssert.AreSame(entryA1, cache.Get(evalCellA1));
+            ClassicAssert.AreSame(entryB1, cache.Get(evalCellB1));
+            ClassicAssert.AreEqual(2, cache.GetCacheEntries().Length);
+
+            // Remove A1 and verify it's actually evicted
+            FormulaCellCacheEntry removed = cache.Remove(evalCellA1);
+            ClassicAssert.AreSame(entryA1, removed, "Remove should return the evicted entry");
+            ClassicAssert.IsNull(cache.Get(evalCellA1), "Entry should be evicted after Remove");
+            ClassicAssert.AreSame(entryB1, cache.Get(evalCellB1), "Other entries should be unaffected");
+            ClassicAssert.AreEqual(1, cache.GetCacheEntries().Length);
+
+            wb.Close();
+        }
+
     }
 
 }