Skip to content

Stop building a million nodes for a million-element array - #53

Merged
ww-mw merged 1 commit into
mainfrom
cap-huge-array-expansion
Oct 1, 2026
Merged

ww-mw merged 1 commit into
mainfrom
cap-huge-array-expansion

Conversation

@ww-mw

@ww-mw ww-mw commented Oct 1, 2026

Copy link
Copy Markdown
Member

A .sldd holding a 1000x1000 double did not open in the VS Code extension: blank tab, extension host pinned for ~2.5 minutes, then a misleading Failed to parse ...: Maximum call stack size exceeded. The parse was never the problem — it took one second. The ingest then built one node per element.

Measured on that file, before: 1,001,276 nodes, ~690 MB for the single entry, ~161 s in element labels alone, and a consumer's table projection of ~413 MB of rows. After: 1,276 nodes, 95 MB, 288 ms.

Two causes, both in this repo

1. Unbounded expansion. An array past the new MAX_EXPANDED_ELEMENTS (10,000) expands into no element children; the value lives in _elements, which every reader already falls back to.

  • 10,000 and not a rounder 4,096 because it must clear a consumer's own grid cap: the extension grids a matrix only when children.length === elementCount, at most 4,096. A cap at or below that would silently kill the grid for exactly the matrices it exists for. Above it the two limits compose — <=4096 rows + grid, <=10,000 rows, past that the summary alone.
  • All-or-nothing by necessity: _elements and the children are two copies of one value, and every reader spells the choice children.length > 0 ? children.map(...) : _elements. A partial expansion would be read through its children and would save the first N elements as the whole value.
  • Verified on the real file: the capped entry serializes back byte-identical to the 4 MB of value text the file itself holds — all 1,000,000 elements, zero differing — and still renders <1000x1000 double>.
  • Deliberately not a ParseWarning, by ParseWarning's own rule: the value was read and is saved completely, so nothing was lost to report.
  • Cells are exempt, and the builder says why: a cell's children are its only copy, so capping there would make _serializeCellXml write Dimension="0*0".

2. Quadratic element labels. BaseNode.displayName derived an element's subscript from parent.children.indexOf(this) — O(n) per element, O(n^2) per array: 1.5 us at index 0 rising to 325 us at index 999,999. The struct-element path eight lines below already read a stored index and was 2000x faster on the same data. Elements now read their slot from the 1-based name every builder stamps and verify it holds this node before trusting it, falling back to the scan when children were reordered without reindexing.

The cap needed one home

Six parse paths wrote their own element loop, with the length > 1 guard spelled three times and missing three times. One of them already carried a comment saying every element builder should state the rule identically — by convention, which held right up until there were two rules to agree on, and a cap honoured by five builders out of six is not a cap. All six now call _buildArrayChildren, with an element-class override for the one container whose elements are not of its own class (a complex array stores _scalarType double; its elements are complex).

Tests

test/largeArrayNotExpanded.test.ts, 10 tests: the cap clears 4,096; past it no children; Value, serializeXml and displayValue stay complete (no Dimension="0*0"); the boundary is inclusive; a cell past the cap still expands; labels correct at a high index in a non-square array and when a child sits at a slot its name does not predict; and the fast path does no scan — asserted as a count, not a clock.

npm run verify green: 5,122 tests, plus typecheck, build, pack, leak and browser-safety checks.

No consumer change is required by this PR. The host-side rows.push(...) argument overflow that produced the user-visible message is a separate fix in the extension repo.

A dictionary holding a 1000x1000 double did not open. The parse was never the
problem — it took a second — but the ingest then built one node per element:
1,000,001 nodes and ~690 MB for that one entry, whose labels alone took ~161 s,
and a consumer's table projection of the subtree was ~413 MB of rows.

Two causes, both here.

An array past MAX_EXPANDED_ELEMENTS (10,000) now expands into NO element
children. The cap has to clear a consumer's own grid cap — the vscode extension
grids a matrix only when it has one child per element, at most 4096 — so the two
limits compose rather than cancel: <=4096 gets rows and a grid, <=10,000 gets
rows, past that the summary alone. It is all-or-nothing by necessity: `_elements`
and the children are two copies of one value and every reader spells the choice
`children.length > 0 ? children.map(...) : _elements`, so a partial expansion
would be read through its children and would save the first N elements as the
whole value. Checked on the file that prompted this: the capped entry serializes
back byte-identical to the 4 MB of value text the file itself holds, all
1,000,000 elements, and still summarizes as `<1000x1000 double>`. Not a
ParseWarning, by ParseWarning's own rule — nothing was lost to report.

Cells are exempt, and say why at the builder: a cell's children are its ONLY
copy, so a cap there would make _serializeCellXml write `Dimension="0*0"`.

The cap also needed somewhere to live. Six parse paths wrote their own element
loop, with the `length > 1` guard spelled three times and missing three times;
the comment at one of them already said every element builder should state the
rule identically, but said it by convention, which lasted exactly until there
were two rules to agree on. All six now call _buildArrayChildren, the only place
an array grows children, with an element-class override for the one container
whose elements are not of its own class.

Second cause: an element's label derived its subscript from
`parent.children.indexOf(this)`, O(n) per element and so O(n^2) per array — 1.5
us at index 0, 325 us at index 999,999. The struct-element path eight lines below
already read a stored index and was 2000x faster on the same data. Elements now
read their slot from the 1-based name every builder stamps, and VERIFY it holds
this node before trusting it, falling back to the scan when children have been
reordered without reindexing. Both halves are pinned, the fast one by a scan
count rather than a clock.

Ingest of that file: 1,001,276 nodes to 1,276, 690 MB to 95 MB, 288 ms total.
@ww-mw
ww-mw merged commit aeb57b6 into main Oct 1, 2026
4 checks passed
@ww-mw
ww-mw deleted the cap-huge-array-expansion branch October 1, 2026 19:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant