Skip to content

Commit 04180f8

Browse files
Fix #1629: resolve memory leaks in LazilyTransformingAstService and UndoRedo (#1638)
## Summary Fixes #1629. Closes #1633. Closes #1634. Two unbounded memory leaks in long-running HyperFormula instances: - **`LTAS.transformations[]`** grew linearly with every structural operation (addRows, removeRows, moveCells, etc.) and was never cleaned up. Fixed by introducing **threshold-based compaction** with `versionOffset` — once 50+ transformations accumulate, all consumers (FormulaVertex, ColumnIndex) are force-updated, then the array is released while the logical version remains monotonically increasing. - **`UndoRedo.oldData`** grew linearly even when entries were evicted from the undo stack. Fixed by tracking which LTAS versions each `UndoEntry` references (`getReferencedOldDataVersions()`), cleaning up on eviction/clear, guarding against writes when `undoLimit === 0`, and running orphan cleanup after compaction to handle a race condition where lazy-apply re-inserts already-evicted keys. ### Changed files | File | Change | |------|--------| | `LazilyTransformingAstService.ts` | `versionOffset`, `compact()`, `needsCompaction()` with threshold=50, offset-aware iteration | | `UndoRedo.ts` | `getReferencedOldDataVersions()` on interface + 7 subclasses, eviction cleanup, `undoLimit===0` guard, `cleanupOrphanedOldData()`, `forceApply` parity in `undoMoveRows`/`undoMoveColumns` | | `HyperFormula.ts` | Compaction trigger in `recomputeIfDependencyGraphNeedsIt()` | | `ColumnIndex.ts` | `forceApplyPostponedTransformations()` — iterates all ValueIndex entries | | `ColumnBinarySearch.ts` | No-op `forceApplyPostponedTransformations()` | | `SearchStrategy.ts` | New method on `ColumnSearchStrategy` interface | | `Operations.ts` | Added `columnSearch.forceApply` to undo path (no compact — centralized in HyperFormula.ts) | ### What is NOT fixed here - **Parser cache** (`ParserWithCaching`) — unbounded growth tracked separately in #1635 ## Test plan - 18 dedicated tests in `hyperformula-tests` — see companion PR in that repo - Full test suite: **480 suites / 5396 tests passed** - Benchmark validated threshold=50 as optimal (eager compaction is ~18× slower on a 2.5k formula sheet) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Touches core recalculation/transform and undo/redo paths; while aimed at memory safety, compaction/cleanup timing could affect formula correctness or undo behavior in edge cases. > > **Overview** > Prevents unbounded memory growth in long-running engines by **adding threshold-based compaction** of lazy formula transformations and by **cleaning up undo snapshot (`oldData`) entries** when undo/redo stack entries are cleared or evicted. > > Introduces new config `maxPendingLazyTransformations` (default `50`) and wires it into engine construction; when the threshold is reached, `HyperFormula` forces pending transformations to be applied (dependency graph + column search), compacts the transformation history, and prunes orphaned `UndoRedo.oldData`. Column search strategies now expose `forceApplyPostponedTransformations()` (real implementation for `ColumnIndex`, no-op for binary search), and undo for move operations ensures postponed transformations are applied before restoring old data. Documentation and changelog are updated accordingly. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit d8ebe1d. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Kuba Sekowski <jakub.sekowski@handsontable.com>
1 parent 0c1d130 commit 04180f8

12 files changed

Lines changed: 304 additions & 8 deletions

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,13 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
99

1010
### Added
1111

12+
- Added `maxPendingLazyTransformations` configuration option to control memory usage by limiting accumulated transformations before cleanup. [#1629](https://github.com/handsontable/hyperformula/issues/1629)
1213
- Added a new function: TEXTJOIN. [#1640](https://github.com/handsontable/hyperformula/pull/1640)
1314

1415
### Fixed
1516

17+
- Fixed a memory leak in `LazilyTransformingAstService` where the transformations array grew unboundedly, causing increasing memory usage over time. [#1629](https://github.com/handsontable/hyperformula/issues/1629)
18+
- Fixed a memory leak in `UndoRedo` where `oldData` entries for evicted undo stack entries were never cleaned up, causing increasing memory usage over time. [#1629](https://github.com/handsontable/hyperformula/issues/1629)
1619
- Fixed the IRR function returning `#NUM!` error when the initial investment significantly exceeds the sum of returns. [#1628](https://github.com/handsontable/hyperformula/issues/1628)
1720
- Fixed the ADDRESS function ignoring `defaultValue` when arguments are syntactically empty (e.g., `=ADDRESS(2,3,,FALSE())`). [#1632](https://github.com/handsontable/hyperformula/issues/1632)
1821

docs/guide/performance.md

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,29 @@ cells filled, but located very far from each other.
3737
the fill ratio of the sheet. Let the engine choose the best strategy
3838
for you.
3939

40+
## Lazy transformation cleanup
41+
42+
Structural operations (adding/removing rows/columns, moving cells) create
43+
transformations that are applied lazily to formulas. Over time, these
44+
transformations accumulate in memory. HyperFormula automatically flushes
45+
them when their count reaches the `maxPendingLazyTransformations` threshold
46+
(default: 50).
47+
48+
You can tune this setting to balance memory usage and CPU overhead:
49+
50+
* **Lower values** (e.g., 10) reduce peak memory usage but trigger
51+
cleanup more frequently, adding slight CPU overhead per flush.
52+
* **Higher values** (e.g., 200) reduce the frequency of cleanup but
53+
allow more memory to accumulate between flushes.
54+
* The default of **50** works well for most use cases.
55+
56+
```javascript
57+
const hf = HyperFormula.buildEmpty({
58+
licenseKey: 'gpl-v3',
59+
maxPendingLazyTransformations: 100,
60+
})
61+
```
62+
4063
## Suspending automatic recalculations
4164

4265
By default, HyperFormula recalculates formulas after every change.

src/BuildEngineFactory.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ export class BuildEngineFactory {
7272

7373
const namedExpressions = new NamedExpressions()
7474
const functionRegistry = new FunctionRegistry(config)
75-
const lazilyTransformingAstService = new LazilyTransformingAstService(stats)
75+
const lazilyTransformingAstService = new LazilyTransformingAstService(stats, config.maxPendingLazyTransformations)
7676
const dependencyGraph = DependencyGraph.buildEmpty(lazilyTransformingAstService, config, functionRegistry, namedExpressions, stats)
7777
const columnSearch = buildColumnSearchStrategy(dependencyGraph, config, stats)
7878
const sheetMapping = dependencyGraph.sheetMapping

src/Config.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ export class Config implements ConfigParams, ParserConfig {
6262
timeFormats: ['hh:mm', 'hh:mm:ss.sss'],
6363
thousandSeparator: '',
6464
undoLimit: 20,
65+
maxPendingLazyTransformations: 50,
6566
useRegularExpressions: false,
6667
useWildcards: true,
6768
useColumnIndex: false,
@@ -135,6 +136,8 @@ export class Config implements ConfigParams, ParserConfig {
135136
/** @inheritDoc */
136137
public readonly undoLimit: number
137138
/** @inheritDoc */
139+
public readonly maxPendingLazyTransformations: number
140+
/** @inheritDoc */
138141
public readonly context: unknown
139142

140143
/**
@@ -198,6 +201,7 @@ export class Config implements ConfigParams, ParserConfig {
198201
useArrayArithmetic,
199202
useStats,
200203
undoLimit,
204+
maxPendingLazyTransformations,
201205
useColumnIndex,
202206
useRegularExpressions,
203207
useWildcards,
@@ -244,10 +248,12 @@ export class Config implements ConfigParams, ParserConfig {
244248
this.nullDate = configValueFromParamCheck(nullDate, instanceOfSimpleDate, 'IDate', 'nullDate')
245249
this.leapYear1900 = configValueFromParam(leapYear1900, 'boolean', 'leapYear1900')
246250
this.undoLimit = configValueFromParam(undoLimit, 'number', 'undoLimit')
251+
this.maxPendingLazyTransformations = configValueFromParam(maxPendingLazyTransformations, 'number', 'maxPendingLazyTransformations')
247252
this.useRegularExpressions = configValueFromParam(useRegularExpressions, 'boolean', 'useRegularExpressions')
248253
this.useWildcards = configValueFromParam(useWildcards, 'boolean', 'useWildcards')
249254
this.matchWholeCell = configValueFromParam(matchWholeCell, 'boolean', 'matchWholeCell')
250255
validateNumberToBeAtLeast(this.undoLimit, 'undoLimit', 0)
256+
validateNumberToBeAtLeast(this.maxPendingLazyTransformations, 'maxPendingLazyTransformations', 1)
251257
this.maxRows = configValueFromParam(maxRows, 'number', 'maxRows')
252258
validateNumberToBeAtLeast(this.maxRows, 'maxRows', 1)
253259
this.maxColumns = configValueFromParam(maxColumns, 'number', 'maxColumns')

src/ConfigParams.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -402,6 +402,21 @@ export interface ConfigParams {
402402
* @category Undo and Redo
403403
*/
404404
undoLimit: number,
405+
/**
406+
* Controls memory usage for long-running instances by limiting the number of
407+
* pending lazy transformations before cleanup occurs.
408+
*
409+
* Structural operations (adding/removing rows/columns, moving cells) create
410+
* transformations that are applied lazily to formulas. This setting determines
411+
* how many can accumulate before they are flushed and memory is reclaimed.
412+
*
413+
* Lower values reduce peak memory usage but may slightly increase CPU overhead.
414+
* Higher values reduce overhead but allow more memory accumulation.
415+
*
416+
* @default 50
417+
* @category Engine
418+
*/
419+
maxPendingLazyTransformations: number,
405420
/**
406421
* When set to `true`, criteria in functions (SUMIF, COUNTIF, ...) are allowed to use regular expressions.
407422
* @default false

src/HyperFormula.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4595,6 +4595,20 @@ export class HyperFormula implements TypedEmitter {
45954595
this._functionRegistry = newEngine.functionRegistry
45964596
}
45974597

4598+
/**
4599+
* When enough transformations have accumulated, forces all formula vertices and
4600+
* column index entries to apply pending lazy transformations, then compacts the
4601+
* transformation history and cleans up orphaned undo oldData entries.
4602+
*/
4603+
private compactLazyTransformationsIfNeeded(): void {
4604+
if (this._lazilyTransformingAstService.needsCompaction()) {
4605+
this._dependencyGraph.forceApplyPostponedTransformations()
4606+
this._columnSearch.forceApplyPostponedTransformations()
4607+
this._lazilyTransformingAstService.compact()
4608+
this._lazilyTransformingAstService.undoRedo?.cleanupOrphanedOldData()
4609+
}
4610+
}
4611+
45984612
/**
45994613
* Runs a recomputation starting from recently changed vertices.
46004614
*
@@ -4610,6 +4624,8 @@ export class HyperFormula implements TypedEmitter {
46104624
const verticesToRecomputeFrom = this.dependencyGraph.verticesToRecompute()
46114625
this.dependencyGraph.clearDirtyVertices()
46124626

4627+
this.compactLazyTransformationsIfNeeded()
4628+
46134629
if (verticesToRecomputeFrom.length > 0) {
46144630
changes.addAll(this.evaluator.partialRun(verticesToRecomputeFrom))
46154631
}

src/LazilyTransformingAstService.ts

Lines changed: 64 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,21 +11,56 @@ import {StatType} from './statistics'
1111
import {Statistics} from './statistics/Statistics'
1212
import {UndoRedo} from './UndoRedo'
1313

14+
/**
15+
* Manages lazy application of formula AST transformations.
16+
*
17+
* ## Problem
18+
* Structural operations (adding/removing rows/columns, moving cells, renaming sheets)
19+
* require updating every formula that references the affected area. Applying these
20+
* transformations eagerly to all formulas after every operation is expensive, especially
21+
* for large spreadsheets with many formulas.
22+
*
23+
* ## Solution: Lazy Transformation
24+
* Instead of transforming all formulas immediately, this service stores transformations
25+
* in a queue. Each formula vertex (FormulaVertex) and column index entry (ValueIndex)
26+
* tracks its own version number. When a consumer needs up-to-date data, it calls
27+
* `applyTransformations()` with its current version and receives all transformations
28+
* accumulated since that version.
29+
*
30+
* ## Compaction
31+
* Over time, the transformations array grows unboundedly. To prevent this memory leak,
32+
* the engine periodically triggers compaction when the number of accumulated
33+
* transformations reaches the configurable `maxPendingLazyTransformations`:
34+
*
35+
* 1. All FormulaVertex instances are forced to apply pending transformations
36+
* (via `DependencyGraph.forceApplyPostponedTransformations()`).
37+
* 2. All ColumnIndex entries are forced to apply pending transformations
38+
* (via `ColumnSearchStrategy.forceApplyPostponedTransformations()`).
39+
* 3. `compact()` is called, which advances `versionOffset` and clears the
40+
* transformations array.
41+
* 4. `UndoRedo.cleanupOrphanedOldData()` removes any oldData entries that were
42+
* written during forced application but belong to already-evicted undo entries.
43+
*
44+
* The `versionOffset` ensures that version numbers remain globally consistent
45+
* after compaction: `version() = versionOffset + transformations.length`.
46+
*/
1447
export class LazilyTransformingAstService {
1548

1649
public parser?: ParserWithCaching
1750
public undoRedo?: UndoRedo
1851

1952
private transformations: FormulaTransformer[] = []
53+
private versionOffset: number = 0
2054
private combinedTransformer?: CombinedTransformer
2155

2256
constructor(
2357
private readonly stats: Statistics,
58+
private readonly maxPendingLazyTransformations: number,
2459
) {
2560
}
2661

2762
public version(): number {
28-
return this.transformations.length
63+
return this.versionOffset + this.transformations.length
2964
}
3065

3166
public addTransformation(transformation: FormulaTransformer): number {
@@ -53,8 +88,9 @@ export class LazilyTransformingAstService {
5388
public applyTransformations(ast: Ast, address: SimpleCellAddress, version: number): [Ast, SimpleCellAddress, number] {
5489
this.stats.start(StatType.TRANSFORM_ASTS_POSTPONED)
5590

56-
for (let v = version; v < this.transformations.length; v++) {
57-
const transformation = this.transformations[v]
91+
const currentVersion = this.version()
92+
for (let v = Math.max(version, this.versionOffset); v < currentVersion; v++) {
93+
const transformation = this.transformations[v - this.versionOffset]
5894
if (transformation.isIrreversible()) {
5995
this.undoRedo!.storeDataForVersion(v, address, this.parser!.computeHashFromAst(ast))
6096
this.parser!.rememberNewAst(ast)
@@ -67,15 +103,37 @@ export class LazilyTransformingAstService {
67103
const cachedAst = this.parser!.rememberNewAst(ast)
68104

69105
this.stats.end(StatType.TRANSFORM_ASTS_POSTPONED)
70-
return [cachedAst, address, this.transformations.length]
106+
return [cachedAst, address, currentVersion]
71107
}
72108

73109
public* getTransformationsFrom(version: number, filter?: (transformation: FormulaTransformer) => boolean): IterableIterator<FormulaTransformer> {
74-
for (let v = version; v < this.transformations.length; v++) {
75-
const transformation = this.transformations[v]
110+
const currentVersion = this.version()
111+
for (let v = Math.max(version, this.versionOffset); v < currentVersion; v++) {
112+
const transformation = this.transformations[v - this.versionOffset]
76113
if (!filter || filter(transformation)) {
77114
yield transformation
78115
}
79116
}
80117
}
118+
119+
/**
120+
* Returns true when enough transformations have accumulated to justify the cost
121+
* of forcing all consumers (FormulaVertex, ColumnIndex) to apply pending changes.
122+
*/
123+
public needsCompaction(): boolean {
124+
return this.transformations.length >= this.maxPendingLazyTransformations
125+
}
126+
127+
/**
128+
* Compacts the transformations array by discarding all entries that have already
129+
* been applied by every consumer. Safe to call only after all FormulaVertex and
130+
* ColumnIndex consumers have been brought up to the current version.
131+
* After calling, UndoRedo.cleanupOrphanedOldData() must be invoked to remove
132+
* oldData entries written during forceApplyPostponedTransformations for
133+
* already-evicted undo entries.
134+
*/
135+
public compact(): void {
136+
this.versionOffset += this.transformations.length
137+
this.transformations = []
138+
}
81139
}

src/Lookup/ColumnBinarySearch.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,16 @@ export class ColumnBinarySearch extends AdvancedFind implements ColumnSearchStra
5353
public removeValues(range: IterableIterator<[RawScalarValue, SimpleCellAddress]>): void {
5454
}
5555

56+
/**
57+
* No-op: ColumnBinarySearch reads cell values directly from the dependency graph
58+
* on every lookup, so it has no cached data that could become stale.
59+
* Unlike ColumnIndex, which maintains a separate value-to-address index that
60+
* must be kept in sync with lazy transformations, binary search always operates
61+
* on the current graph state.
62+
*/
63+
public forceApplyPostponedTransformations(): void {
64+
}
65+
5666
/*
5767
* WARNING: Finding lower/upper bounds in unordered ranges is not supported. When ordering === 'none', assumes matchExactly === true
5868
*/

src/Lookup/ColumnIndex.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,24 @@ export class ColumnIndex implements ColumnSearchStrategy {
184184
this.index.delete(sheetId)
185185
}
186186

187+
/**
188+
* Forces all ValueIndex entries to apply any pending lazy transformations,
189+
* bringing every entry up to the current LazilyTransformingAstService version.
190+
* Must be called before compacting LazilyTransformingAstService.
191+
*/
192+
public forceApplyPostponedTransformations(): void {
193+
for (const [sheet, sheetIndex] of this.index) {
194+
sheetIndex.forEach((columnMap, col) => {
195+
if (!columnMap) {
196+
return
197+
}
198+
for (const value of columnMap.keys()) {
199+
this.ensureRecentData(sheet, col, value)
200+
}
201+
})
202+
}
203+
}
204+
187205
public getColumnMap(sheet: number, col: number): ColumnMap {
188206
if (!this.index.has(sheet)) {
189207
this.index.set(sheet, [])

src/Lookup/SearchStrategy.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,13 @@ export interface ColumnSearchStrategy extends SearchStrategy {
5151
moveValues(range: IterableIterator<[RawScalarValue, SimpleCellAddress]>, toRight: number, toBottom: number, toSheet: number): void,
5252

5353
removeValues(range: IterableIterator<[RawScalarValue, SimpleCellAddress]>): void,
54+
55+
/**
56+
* Forces all lazily-tracked ValueIndex entries to apply any pending transformations,
57+
* bringing every entry's version up to the current LazilyTransformingAstService version.
58+
* Must be called before compacting LazilyTransformingAstService.
59+
*/
60+
forceApplyPostponedTransformations(): void,
5461
}
5562

5663
export function buildColumnSearchStrategy(dependencyGraph: DependencyGraph, config: Config, statistics: Statistics): ColumnSearchStrategy {

0 commit comments

Comments
 (0)