Merge pull request #201 from anmcgrath/improve-formula-performance

Improve formula performance by using caching of computed values.

Author: anmcgrath

src/BlazorDatasheet.Core/Commands/Data/SetCellValuesCommand.cs

@@ -7,11 +7,7 @@ namespace BlazorDatasheet.Core.Commands.Data;
 
 public class SetCellValuesCommand : BaseCommand, IUndoableCommand
 {
-    private readonly object[][]? _values;
-    private readonly CellValue[][]? _cellValues;
-    private readonly IRegion? _region;
-    private readonly object? _singleValue;
-    private readonly CellValue? _singleCellValue;
+    private readonly IEnumerable<IEnumerable<CellValue>> _cellValues;
     private readonly int _row;
     private readonly int _col;
     private readonly CellStoreRestoreData _restoreData = new();
@@ -22,46 +18,37 @@ public class SetCellValuesCommand : BaseCommand, IUndoableCommand
     /// <param name="values">The values to set. values[row] is the row offset from <paramref name="row"/>. Each values[row][col] is the col offset from <paramref name="col"/></param>
     /// <param name="row"></param>
     /// <param name="col"></param>
-    public SetCellValuesCommand(int row, int col, object[][] values)
+    public SetCellValuesCommand(int row, int col, CellValue[][] values)
     {
-        _values = values;
+        _cellValues = values;
         _row = row;
         _col = col;
     }
 
+
     /// <summary>
     /// Creates a command to set multiple cell values starting at position <paramref name="row"/>/<paramref name="col"/>
     /// </summary>
-    /// <param name="values">The values to set. values[row] is the row offset from <paramref name="row"/>. Each values[row][col] is the col offset from <paramref name="col"/></param>
     /// <param name="row"></param>
     /// <param name="col"></param>
-    public SetCellValuesCommand(int row, int col, CellValue[][] values)
+    /// <param name="values"></param>
+    public SetCellValuesCommand(int row, int col, IEnumerable<IEnumerable<CellValue>> values)
     {
         _cellValues = values;
         _row = row;
         _col = col;
     }
 
-    /// <summary>
-    /// Sets all the cells in the region <paramref name="region"/> to <paramref name="value"/>
-    /// </summary>
-    /// <param name="region"></param>
-    /// <param name="value"></param>
-    public SetCellValuesCommand(IRegion region, object? value)
-    {
-        _region = region;
-        _singleValue = value;
-    }
-
     /// <summary>
     /// Sets all the cells in the region <paramref name="region"/> to <paramref name="value"/>
     /// </summary>
     /// <param name="region"></param>
     /// <param name="value"></param>
     public SetCellValuesCommand(IRegion region, CellValue value)
     {
-        _region = region;
-        _singleCellValue = value;
+        _row = region.Top;
+        _col = region.Left;
+        _cellValues = Enumerable.Repeat(Enumerable.Repeat(value, region.Width), region.Height);
     }
 
     public override bool CanExecute(Sheet sheet) => true;
@@ -70,85 +57,39 @@ public override bool Execute(Sheet sheet)
     {
         sheet.ScreenUpdating = false;
         sheet.BatchUpdates();
-        if (_values != null)
-            ExecuteSetObjectArrayData(sheet);
-        else if (_cellValues != null)
-            ExecuteSetCellValueData(sheet);
-        else if (_region != null && _singleCellValue != null)
-            ExecuteSetRegionDataAsCellValue(sheet);
-        else if (_region != null)
-            ExecuteSetRegionData(sheet);
-        else
-            return false;
+        ExecuteSetCellValueData(sheet);
         sheet.EndBatchUpdates();
         sheet.ScreenUpdating = true;
 
         return true;
     }
 
-    private void ExecuteSetObjectArrayData(Sheet sheet)
-    {
-        var rowEnd = _row + _values!.Length;
-        var colEnd = _col;
-
-        for (int i = 0; i < _values.Length; i++)
-        {
-            for (int j = 0; j < _values[i].Length; j++)
-            {
-                _restoreData.Merge(sheet.Cells.SetValueImpl(_row + i, _col + j, _values[i][j]));
-                colEnd = Math.Max(colEnd, j + _col);
-            }
-        }
-
-        sheet.MarkDirty(new Region(_row, rowEnd, _col, colEnd));
-    }
-
     private void ExecuteSetCellValueData(Sheet sheet)
     {
-        var rowEnd = _row + _cellValues!.Length;
+        var rowEnd = _row;
         var colEnd = _col;
 
-        for (int i = 0; i < _cellValues.Length; i++)
-        {
-            for (int j = 0; j < _cellValues[i].Length; j++)
-            {
-                _restoreData.Merge(sheet.Cells.SetValueImpl(_row + i, _col + j, _cellValues[i][j]));
-                colEnd = Math.Max(colEnd, j + _col);
-            }
-        }
+        int rowOffset = 0;
 
-        sheet.MarkDirty(new Region(_row, rowEnd, _col, colEnd));
-    }
-
-
-    private void ExecuteSetRegionData(Sheet sheet)
-    {
-        for (int row = _region!.Top; row <= _region!.Bottom; row++)
+        foreach (var row in _cellValues)
         {
-            for (int col = _region.Left; col <= _region.Right; col++)
+            int colOffset = 0;
+            
+            foreach (var value in row)
             {
-                _restoreData.Merge(sheet.Cells.SetValueImpl(row, col, _singleValue));
+                _restoreData.Merge(sheet.Cells.SetValueImpl(_row + rowOffset, _col + colOffset, value));
+                colEnd = Math.Max(colEnd, _col + colOffset);
+                colOffset++;
             }
-        }
-
-        sheet.MarkDirty(_region);
-    }
-
-    private void ExecuteSetRegionDataAsCellValue(Sheet sheet)
-    {
-        sheet.ScreenUpdating = false;
 
-        for (int row = _region!.Top; row <= _region!.Bottom; row++)
-        {
-            for (int col = _region.Left; col <= _region.Right; col++)
-            {
-                _restoreData.Merge(sheet.Cells.SetValueImpl(row, col, _singleCellValue!));
-            }
+            rowEnd = Math.Max(rowEnd, _row + rowOffset);
+            rowOffset++;
         }
 
-        sheet.MarkDirty(_region);
+        sheet.MarkDirty(new Region(_row, rowEnd, _col, colEnd));
     }
 
+
     public bool Undo(Sheet sheet)
     {
         sheet.ScreenUpdating = false;

src/BlazorDatasheet.Core/Data/Cells/CellStore.Data.cs

@@ -25,7 +25,7 @@ public partial class CellStore
     /// <param name="col"></param>
     /// <param name="value"></param>
     /// <returns></returns>
-    public bool SetValue(int row, int col, object value)
+    public bool SetValue(int row, int col, object? value)
     {
         var cmd = new SetCellValueCommand(row, col, ConvertToCellValue(row, col, value));
         return Sheet.Commands.ExecuteCommand(cmd);
@@ -100,7 +100,9 @@ internal CellStoreRestoreData SetValueImpl(int row, int col, object? value)
     /// <returns></returns>
     public bool SetValues(int row, int col, object[][] values)
     {
-        var cmd = new SetCellValuesCommand(row, col, values);
+        var cmd = new SetCellValuesCommand(row, col,
+            values.Select((r, rowOffset) =>
+                r.Select((v, colOffset) => ConvertToCellValue(row + rowOffset, col + colOffset, v))));
         return Sheet.Commands.ExecuteCommand(cmd);
     }
 
@@ -123,8 +125,20 @@ public bool SetValues(int row, int col, CellValue[][] values)
     /// <returns></returns>
     public bool SetValues(IRegion region, object? value)
     {
-        var cmd = new SetCellValuesCommand(region, value);
-        return Sheet.Commands.ExecuteCommand(cmd);
+        if (region.IsSingleCell())
+            return SetValue(region.Top, region.Left, value);
+
+        var cellValues = new CellValue[region.Height][];
+        for (int i = 0; i < region.Height; i++)
+        {
+            cellValues[i] = new CellValue[region.Width];
+            for (int j = 0; j < region.Width; j++)
+            {
+                cellValues[i][j] = ConvertToCellValue(i + region.Top, j + region.Left, value);
+            }
+        }
+
+        return SetValues(region.Top, region.Left, cellValues);
     }
 
     /// <summary>

src/BlazorDatasheet.Core/Data/Cells/CellStore.Validation.cs

@@ -22,7 +22,7 @@ internal void ValidateRegion(IRegion region)
             _validStore.Set(row, col, result.IsValid);
         }
 
-        Sheet.MarkDirty(cellsAffected);
+        Sheet.MarkDirty(region);
     }
 
     public bool IsValid(int row, int col)

src/BlazorDatasheet.Core/Data/Sheet.cs

@@ -154,9 +154,9 @@ public bool ScreenUpdating
     public NamedRangeManager NamedRanges { get; }
 
     /// <summary>
-    /// If the sheet is batching dirty regions, they are stored here.
+    /// If the sheet is batching dirty rows, they are stored here.
     /// </summary>
-    private readonly ConsolidatedDataStore<bool> _dirtyRegions = new();
+    private readonly Range1DStore<bool> _dirtyRows = new(false);
 
     private Sheet(int defaultWidth, int defaultHeight)
     {
@@ -329,15 +329,6 @@ public bool IsCellVisible(int row, int col)
         return null;
     }
 
-    /// <summary>
-    /// Mark the cells specified by positions dirty.
-    /// </summary>
-    /// <param name="positions"></param>
-    internal void MarkDirty(IEnumerable<CellPosition> positions)
-    {
-        MarkDirty(positions.Select(p => new Region(p.row, p.col)));
-    }
-
     /// <summary>
     /// Marks the cell as dirty and requiring re-render
     /// </summary>
@@ -358,7 +349,9 @@ internal void MarkDirty(IRegion region)
         if (intersection == null)
             return;
 
-        MarkDirty(new List<IRegion>() { intersection });
+        _dirtyRows.Set(region.Top, region.Bottom, true);
+        if (!_isBatchingChanges)
+            EmitSheetDirty();
     }
 
     /// <summary>
@@ -373,17 +366,20 @@ internal void MarkDirty(IEnumerable<IRegion> regions)
             if (intersection == null)
                 continue;
 
-            _dirtyRegions.Add(intersection, true);
+            _dirtyRows.Set(region.Top, region.Bottom, true);
         }
 
         if (!_isBatchingChanges)
+            EmitSheetDirty();
+    }
+
+    private void EmitSheetDirty()
+    {
+        SheetDirty?.Invoke(this, new()
         {
-            SheetDirty?.Invoke(this, new DirtySheetEventArgs()
-            {
-                DirtyRegions = _dirtyRegions,
-            });
-            _dirtyRegions.Clear();
-        }
+            DirtyRows = _dirtyRows
+        });
+        _dirtyRows.Clear();
     }
 
     private int _batchRequestNo;
@@ -400,7 +396,7 @@ public void BatchUpdates()
         if (_isBatchingChanges)
             return;
 
-        _dirtyRegions.Clear();
+        _dirtyRows.Clear();
         Cells.BatchChanges();
         _isBatchingChanges = true;
     }
@@ -414,9 +410,12 @@ public void SortRange(IRegion? region, List<ColumnSortOptions>? sortOptions = nu
 
         var beforeArgs = new BeforeRangeSortEventArgs(region, sortOptions);
         BeforeRangeSort?.Invoke(this, beforeArgs);
+
         var cmd = new SortRangeCommand(region, sortOptions);
+
         if (!beforeArgs.Cancel)
             Commands.ExecuteCommand(cmd);
+
         var afterArgs = new RangeSortedEventArgs(region, cmd.SortedRegion, cmd.OldIndices);
         RangeSorted?.Invoke(this, afterArgs);
     }
@@ -435,16 +434,10 @@ public void EndBatchUpdates()
 
         // Checks for batching changes here, because the cells changed event
         // may start batching more dirty changes again from subscribers of that event.
-        if (_dirtyRegions.Any() && _isBatchingChanges)
-        {
-            SheetDirty?.Invoke(this, new DirtySheetEventArgs()
-            {
-                DirtyRegions = _dirtyRegions,
-            });
-        }
+        if (_dirtyRows.Any() && _isBatchingChanges)
+            EmitSheetDirty();
 
         _isBatchingChanges = false;
-        _dirtyRegions.Clear();
     }
 
     /// <summary>

src/BlazorDatasheet.Core/Events/Visual/DirtySheetEventArgs.cs

@@ -5,5 +5,5 @@ namespace BlazorDatasheet.Core.Events.Visual;
 
 public class DirtySheetEventArgs
 {
-    public ConsolidatedDataStore<bool> DirtyRegions { get; init; } = default!;
+    public Range1DStore<bool> DirtyRows { get; init; } = default!;
 }

src/BlazorDatasheet.Core/FormulaEngine/FormulaEngine.cs

@@ -183,14 +183,14 @@ public void CalculateSheet(bool calculateAll)
                 {
                     // check whether the formula has already been calculated in this scc group - may be the case if we lucked
                     // out on the first value calculation and it wasn't a circular reference.
-                    if (scc.Count > 1 && executionContext.TryGetExecutedValue(vertex.Formula, out var result))
+                    if (executionContext.TryGetExecutedValue(vertex.Formula, out var result))
                     {
-                        //TODO: This is never hit so we are always recalculating
                         value = result;
                     }
                     else
                     {
                         value = _evaluator.Evaluate(vertex.Formula, executionContext);
+                        executionContext.RecordExecuted(vertex.Formula, value);
                         if (value.IsError() && ((FormulaError)value.Data!).ErrorType == ErrorType.Circular)
                             isCircularGroup = true;
                     }

src/BlazorDatasheet.DataStructures/Intervals/MergeableIntervalStore.cs

@@ -411,8 +411,8 @@ private void UpdateStartEndPositions()
     {
         if (_intervals.Any())
         {
-            Start = _intervals.First().Value.Start;
-            End = _intervals.Last().Value.End;
+            Start = _intervals.Values[0].Start;
+            End = _intervals.Values[_intervals.Count - 1].End;
         }
     }
 

src/BlazorDatasheet.DataStructures/Store/IStore.cs

@@ -5,7 +5,7 @@ namespace BlazorDatasheet.DataStructures.Store;
 public interface IStore<T, TRestoreData>
 {
     /// <summary>
-    /// Returns whether the the store contains any non-empty data at the row, column specified.
+    /// Returns whether store contains any non-empty data at the row, column specified.
     /// </summary>
     /// <param name="row"></param>
     /// <param name="col"></param>

src/BlazorDatasheet.DataStructures/Store/Range1DStore.cs

@@ -142,6 +142,8 @@ public int GetNextNonEmptyIndex(int index)
         var interval = Intervals.GetNextNonEmptyIndex(index);
         return interval;
     }
+
+    public bool Any() => Intervals.Any();
 }
 
 public class OverwritingValue<R> : IMergeable<OverwritingValue<R>>

src/BlazorDatasheet.Formula.Core/Dependencies/DependencyManager.cs

@@ -270,6 +270,14 @@ public DependencyManagerRestoreData InsertRowColAt(int index, int count, Axis ax
 
     private List<FormulaVertex> GetVerticesInRegion(IRegion region, string sheetName)
     {
+        if (region.IsSingleCell())
+        {
+            var vertex = _dependencyGraph.GetVertex(new FormulaVertex(region, sheetName, null).SheetName);
+            if (vertex != null)
+                return [vertex];
+            return new List<FormulaVertex>();
+        }
+
         var vertices = new List<FormulaVertex>();
         foreach (var v in _dependencyGraph.GetAll())
         {