Skip to content

Admin grid tester: Store memory flags, configurable record shape, and a repeatable benchmark harness - #884

Closed
amcclain wants to merge 19 commits into
developfrom
store-fixed-data-shape
Closed

Admin grid tester: Store memory flags, configurable record shape, and a repeatable benchmark harness#884
amcclain wants to merge 19 commits into
developfrom
store-fixed-data-shape

Conversation

@amcclain

@amcclain amcclain commented Jul 28, 2026

Copy link
Copy Markdown
Member

Companion to xh/hoist-react#4521. Extends the Admin > Tests > Grid tester into a rig for measuring the memory and load-time profile of a large Store, and for producing numbers worth quoting.

Nothing outside Admin > Tests > Grid is touched.

Data shape controls

The tester can now dial in an arbitrary record shape independently of the fields its Store declares, which is what makes the useFixedDataShape criterion (populated fields per record, not declared) measurable at all.

  • Extra Fields / Populate — declare N extra fields, and have the server either populate them or leave them defined-but-null. The latter is the wide-and-sparse shape where useFixedDataShape is a regression.
  • Value Mixmixed, categorical, unique or numeric. All four populate the same ~11/12 of the extra fields, so switching mixes varies value character without moving populated-field count, which would otherwise confound the two effects.
  • Categories — cardinality of the categorical string pool, 1 to 1,000,000. Generated names are fixed-width, so a cardinality sweep varies pool size without also varying value byte size. Disabled for mixes that generate no categorical values.

Both new params are strict server-side: an unknown mix or out-of-range cardinality fails the request rather than defaulting, so a mistyped param cannot serve one dataset under another's label.

Store / fetch flags

Six flags are exposed on their own toolbar row: useFixedDataShape, useRawAsData, freezeData, retainRaw, reuseRecords (all StoreConfig) and internStrings (a FetchOptions config, applied to whichever load path is active). That covers all three record-data representations, so they can be measured against each other on identical data.

  • freezeData is a control rather than a hardcoded false, defaulted to Hoist's own default so measurements reflect what apps actually run.
  • Mutual exclusions are enforced three ways: the incompatible switches disable, a reaction clears them, and Store construction guards again for configs restored from the ViewManager. retainRaw is surfaced as inert under useRawAsData, since record data is the raw object there.
  • Toggling any of the three representation flags reloads the app, behind a confirm dialog that reverts on cancel. This is deliberate and load-bearing: V8 decides property storage per isolate, so measuring two record-data representations in one session contaminates whichever runs second. Settings persist across the reload, so a configured A/B survives it. The remaining flags only change what is loaded or retained and can be flipped in place.

Benchmark harness

Opened from the Store toolbar row. Pick a scenario and iteration count, run, read the results grid, "Copy as Markdown" to export. Results persist to local storage, so an A/B survives the reload the record-data flags force.

Three scenarios: cold load; reload (re-fetch), the only way to observe cross-fetch interning; and reload (same raw refs), the only way reuseRecords can hit, as it matches on raw reference identity.

Each iteration settles the heap, records a baseline, runs one load, settles again, and reports heap delta, bytes/record and load time as min/median/max — never a single number. Uses window.gc when Chrome is launched with --js-flags="--expose-gc", otherwise falls back to allocation pressure, and states which mode it used on every row so a weakly-settled figure is not mistaken for a real one.

Two properties worth calling out, because both exist to stop the harness reporting something false:

  • Baselines are verified, not assumed. A pristine empty-grid baseline is captured once per run; later iterations wait for the heap to return to within tolerance of it before measuring. Iterations that never come back still run but are recorded Suspect, with the residue in MB, a banner, and a Heap Δ #1 column holding the first iteration's delta — the one measured from the pristine baseline by construction, and the only trustworthy figure when the median is not.
  • Concurrent runs are refused outright. Two overlapping runs corrupt both, each clearing the grid under the other, and the suspect check structurally cannot detect it — it only compares a run against its own baseline. Guarded in the model rather than relying on a disabled button.

Rows are self-describing: every flag, record count, declared field count, measured populated-field count, value mix and cardinality. Rows recorded before the value-mix work render ? rather than blank in those columns, with a tooltip noting their byte figures do not compare with later runs.

Reproducibility

Test parameters save as named ViewManager configs, so a given matrix cell can be restored exactly rather than reconstructed from memory.

Merge blockers

Reviewer note

GridTestModel.reloadForRecordDataChange POSTs directly to xhView/updateState — a Hoist Core impl endpoint — to make the ViewManager's fire-and-forget state write durable before reloading the page. It works and is commented, but it reaches past the public client API. The clean fix is upstream: expose that write as an awaitable task on ViewManagerModel. Flagging rather than hiding it; happy to file that and drop this if preferred.


Hoist P/R Checklist

  • Caught up with develop branch as of last change.
  • Added CHANGELOG entry, or determined not required. (Admin test-panel tooling only — no app-facing change.)
  • Reviewed for breaking changes, added breaking-change label + CHANGELOG if so. (None — additive, confined to the grid tester.)
  • Updated doc comments / prop-types, or determined not required.
  • Reviewed and tested on Mobile, or determined not required. (Desktop admin console only.)

amcclain added 17 commits July 22, 2026 13:02
…oggle

Supports measurement/validation of xh/hoist-react#4500 (per-store codegen'd record data factory):

- Added "Populate" switch: fills the tester's extra fields with generated values, allowing stress tests with configurable populated record widths (vs. wide-but-sparse field definitions, the only previous option).
- Added "Legacy Data Objects" switch: disables the store's record data factory via its internal kill switch, reverting to legacy sparse-prototype data objects for side-by-side comparison of memory usage and load/update times.
…anch

The grid tester's `RecordDataFactory` import (xh/hoist-react#4500) is not yet available in a published @xh/hoist snapshot, so `tsc` cannot resolve it against node_modules. Point type-checking at the local inline hoist-react checkout (normally left commented out) rather than suppressing with a ts-ignore, with an in-file note to re-comment once a published build includes the new module.
…dology

The harness was written against the original codegen'd record data factory,
which no longer exists - it imported `RecordDataFactory` and flipped that
class's internal kill switch. Both are gone from hoist-react, so this branch no
longer compiled.

- Switch to the real `Store.optimizeRecordData` config, passed through
  `storeConf`. Removes the import, the kill-switch dance, and the
  `createGridModelInternal` split that existed only to wrap it.
- Invert the toggle to match the new opt-in default-off API: "Legacy Data
  Objects" becomes "Optimize Record Data".
- Toggling now reloads the app rather than rebuilding the grid in place, and the
  setting is persisted so it survives the reload. V8 decides property storage
  per isolate, from a transition tree shared across all code in the page, so
  measuring both representations in one session contaminates whichever runs
  second - in either direction. This produced several false results while
  investigating hoist-react#4501, including an apparent 23x regression and a
  phantom field-count cliff, both of which disappeared under one-shape-per-page
  measurement.
- Update the tsconfig note, which cited `RecordDataFactory` as the reason the
  local hoist-react path override is committed enabled on this branch.
…actory-ab

# Conflicts:
#	client-app/src/admin/tests/grids/GridTestData.ts
…ense-record shape

The move to server-sourced data (#880) left `populateExtraFields` as dead client-side code, so the tester could only produce wide-and-sparse stores (extra fields declared but never populated) - precisely the shape where `Store.optimizeRecordData` loses. Both endpoints can now generate real values for the extra fields, putting the whole populated-fields axis back in reach.

- `GridTestController`: `data` and `streamingData` both accept `extraFieldCount` and `populateExtraFields`. When populating, each row carries `extraField0..N-1` with types fixed per field and mixed as they would be in a real wide grid - repeated categorical strings, unique strings, ints, doubles, bools, and one null column per twelve. Field names are pre-computed once per request, and the streaming path still streams (rows are generated and flushed incrementally, never buffered).
- `GridTestModel`: passes both params through to both endpoints, reviving the panel's "Populate" switch end to end.
- `GridTestModel`: extra field declarations now match the server exactly - the loop was off by one, declaring `extraFieldCount + 1` fields.
- `GridTestPanel`: tooltips for "Extra Fields" and "Populate" describe what each knob does to record shape.

Note that `populateExtraFields` deliberately does NOT trigger an app reload - it changes the data, not the way record data objects are built, so it does not contaminate a V8 hidden-class measurement the way `optimizeRecordData` does.
Toggling `optimizeRecordData` reloads the app, by design - V8 decides property
storage per isolate, so each side of an A/B must be measured in a fresh page.
But `recordCount`, `extraFieldCount`, `populateExtraFields` and
`streamServerLoad` were not persisted, so the reload discarded the very shape
being tested and reset the panel to defaults. Marks them `@persist`.
Replaces the per-property localStorage persistence on GridTestModel with a ViewManagerModel, so a full set of A/B benchmark parameters can be saved under a name, switched between, shared, and reproduced later.

- New `gridTestConfig` ViewManagerModel, created in the admin AppModel.initAsync() so its saved configs are loaded before GridTestModel binds to it. Auto-save disabled - a benchmark config should only change when explicitly saved.
- All panel settings are now `@persist`-ed through that model, including the previously unpersisted `idSeed`, `numericId`, `enableXssProtection`, `twiddleCount` and the grid/chooser/selection options. Unsaved tweaks remain durable across the reload via the ViewManager's own sessionStorage mirror of its pending value.
- ViewManager control added to the top toolbar of the panel.
- Reload-on-`optimizeRecordData` now runs through `reloadForRecordDataChangeAsync()`, which waits for the ViewManager to settle and makes the selected-config record durable before dropping the page - otherwise restoring a config that flips the record-data representation could reload before the selection reached the server and come back on the previous config.
Routine `yarn upgrade`. Realigns React to 19.2.8, matching hoist-react - required
for inline-hoist development, as React 19 throws on a react/react-dom version
mismatch and inline builds resolve modules from both checkouts.
Toggling the setting reloaded the app with no warning, which is jarring and
gives no chance to back out. Now prompts first, explaining that the setting only
takes effect on a fresh page and that runs either side of the change are not
comparable within one session.

Reverts the setting if the user declines, so the switch can never disagree with
the Store actually under test. A guard field swallows the reaction that revert
would otherwise trigger.

Note the dialog is not a synchronization mechanism - it usually gives the
ViewManager's fire-and-forget state write time to land, but that is a timing
accident, so the explicit await ahead of the reload stays.
…rness

Exposes the rest of the Store/fetch flags that drive the memory and load-time profile of a large dataset, and adds a measurement mode that produces numbers worth quoting.

New controls (own "Store:" toolbar row, alongside the existing Optimize Record Data switch):
- `freezeData` - now a control rather than a hardcoded `false`, defaulted to Hoist's own default (true) so measurements reflect what apps actually run.
- `retainRaw`, `reuseRecords` - Store configs, with the illegal `reuseRecords` + `retainRaw: false` pairing prevented in the UI (switch disabled), cleared by a reaction, and guarded again at Store construction for configs restored from the ViewManager.
- `internStrings` - a FetchOption (not a StoreConfig), applied to whichever load path is in use. The interning cache is keyed per dataset and cleared when the switch goes off.

Reload discipline extended to `freezeData` only: like `optimizeRecordData` it changes how record data objects are built and stored, so both sides of an A/B need a fresh page. `retainRaw`, `reuseRecords` and `internStrings` only change what is loaded or retained and can be flipped in place. The confirm/revert handler now covers the set, comparing against the values the page's Stores were built with rather than tracking a revert flag.

New benchmark harness (GridTestBenchmarkModel + dialog, opened from the Store row):
- Each iteration settles the heap, records a baseline, runs one load, settles again and reports delta heap, bytes/record and load time. N iterations per run (default 3), reported as min/median/max - never a single number.
- Uses `window.gc` when available (Chrome with `--js-flags="--expose-gc"`); otherwise falls back to allocation pressure plus the lowest of several spaced reads, and states which mode was used on every row. The dialog surfaces the launch command for the better mode.
- Three scenarios: cold load, reload (re-fetch, the only way to see cross-fetch interning), and reload with the same raw refs (the only way `reuseRecords` can hit, as it matches on reference identity).
- Rows are self-describing - every flag, record count, declared field count and *measured* populated-field count - and accumulate in local storage so an A/B survives the reload the record-data flags force. Copy as markdown for pasting elsewhere.

Reports what was measured only; no derived ratios against other rows.
The harness took each iteration's baseline immediately after clearing the grid, on the assumption that a few gc() passes would reclaim the previous iteration's data. They do not, so every iteration after the first measured from a baseline still holding the last one's records and reported a fraction of the real cost - a 50k-record cold run spanning 20MB to 375MB and presenting a 198MB median.

- Settling with real GC now loops `gc()` until two consecutive reads agree (min 3 passes, cap 25) rather than running a fixed three, and gives React and ag-Grid a beat to unmount and release before it starts.
- A pristine, empty-grid baseline is captured once per run. Each later iteration then waits (polling, 15s cap) for the heap to come back to within tolerance of it before measuring. Tolerance scales with the heap the previous iteration has to give back - 5% of it, floored at 5MB - so a small dataset is not waved through by a bar sized for a large one.
- Iterations that never come back still run, but are recorded suspect and surfaced: a Suspect column carrying the residue in MB, a banner over the results grid, and a `Heap Δ #1` column holding the first iteration's delta, which is measured from the pristine baseline by construction and stands when the median does not.
- Between iterations the grid is now cleared *and* rebuilt, so the next load starts against a Store with no history - `reuseRecords` would otherwise carry its record cache across.

What this does not do is make the iterations independent, because the app does not release the dataset. Measured on a freshly loaded page at 50k x 157 populated fields, settling fully at each step: empty 377MB -> loaded 752MB -> after clear() 733MB -> after destroying the grid 733MB. Clearing returns ~19MB of 375MB and destroying the grid on top of that returns nothing; the data stays live behind a Store with no records and a destroyed GridModel, and comes back unpredictably during a later load. Something outside the Store and the GridModel is holding the records or their raw rows.

So at this size only the first iteration of a cold run is trustworthy, and the harness now says exactly that rather than averaging it away. Verified with `--js-flags="--expose-gc"`:

- Cold, 50k x 157 (144 populated), N=3: Heap Δ #1 375.3MB / 7,446 bytes per record, reproduced across four runs (375.2-375.3MB) and matching a hand measurement of the same config. Iterations 2 and 3 flagged suspect.
- Reload (same raw refs), 5k, N=3: 0.0MB, 6 bytes per record, 132ms - records reused, no new allocation.
- Reload (re-fetch), 5k, N=3: 36.0MB (31.7-36.0), 7,195 bytes per record, 2,269ms. Bytes per record agrees with the 50k cold figure, an independent check on both.

Note the reload scenarios are largely robust to a contaminated baseline: their two readings bracket a single load within one iteration, so a constant residue cancels out of the delta.
Memory results are sensitive to the character of the data, not just its shape - a figure measured against one value distribution says little about another. The generator's value types were a fixed 12-slot cycle and its categorical pool a fixed 8 names, so that axis could not be varied at all.

Server (`GridTestController`):

- New `valueMix` param selects one of four distributions: `mixed` (the previous cycle), `categorical`, `unique` and `numeric`. Every mix is twelve slots carrying a single null in the same final position, so all four emit the same ~11/12 populated fields per row. That is deliberate - populated-field count is the variable that decides whether `optimizeRecordData` pays, and a mix comparison that also moved it would confound the two effects.
- New `categoryCount` param sets the cardinality of the categorical pool, 1 to 1,000,000. The pool is materialized only when the selected mix actually has categorical slots.
- Generated string values are now fixed-width (`Cat-000000`, `Uni-000000001-000000`), so a cardinality sweep varies pool size without also varying value byte size. Previously `Alpha`..`Hotel` ranged 5-7 chars and unique values 7-14, which would have moved two variables at once. This does change absolute byte figures relative to earlier runs.
- Both params are strict: an unknown mix or an out-of-range cardinality fails the request rather than falling back to a default, so a mistyped param cannot quietly serve one dataset under another's label.
- Zero-padding is hand-rolled rather than `String.format`, as it runs once per generated string value - millions of times on a wide `unique` run.

Client:

- Both exposed on the data-shape toolbar row. The cardinality input is disabled for mixes that generate no categorical values, and both are disabled when extra fields are not populated.
- Neither requires a page reload, unlike the record-data flags: every mix emits the same keys, so the record shape - and hence V8's property-storage decision - is identical across mixes.
- Both are recorded on every benchmark result row, in the results grid and the markdown export. Rows recorded before this change render `?` rather than blank in those columns, with a tooltip noting they were measured against the old variable-width values and that their byte figures do not compare with later runs.

Verified against a running server: all four mixes produce their documented type patterns, `categoryCount: 5` draws only from `Cat-000000`..`Cat-000004`, a single distinct string length is emitted at both cardinality 8 and 5000, all four mixes emit exactly 110 of 120 populated fields, and all three malformed-param cases are rejected.
Two overlapping runs silently corrupt both. Each clears and rebuilds the grid under the other, so one measures against a baseline the other has already dirtied, and one records zero records against a multi-hundred-MB heap delta. Both then land as ordinary-looking result rows.

The suspect check cannot catch this - it only compares a run against its own pristine baseline, so it has no way to notice a second run interfering. Nothing in the recorded row marks it as untrustworthy.

Hit while measuring: two runs started ~11 seconds apart produced a `records: 0` row reporting 650MB and a second row reporting 688MB, where the same configuration measures 453MB cleanly. Both looked plausible enough to quote.

`runBenchmark()` now returns early and logs a warning when a run is already in flight, rather than relying on the dialog's disabled button to be the only barrier.
…actory-ab

Conflict in `GridTestController.streamingData()`, where #882 replaced this branch's hand-rolled NDJSON streaming with Hoist Core's `renderNdjson()`.

Resolved in favour of `renderNdjson()`, keeping this branch's added `valueMix` / `categoryCount` / extra-field params. The buffering, content-type and flush-cadence handling this branch carried - along with its explanatory comments and the `JsonOutput` / `BufferedOutputStream` imports - is now the framework's concern and has been dropped. Row generation feeds `renderNdjson()` via develop's `Generator.flatRows()` iterator, which drives the same `nextParent()` and so still applies generated extra fields.
Follows xh/hoist-react#4501, where `optimizeRecordData` was renamed to `useFixedDataShape`.

- Model bindable, Store config, reload-discipline flag list and benchmark result field all renamed.
- UI label and results column now read "Fixed Data Shape" / "FixedShape".

Note the persisted key changes with the bindable name, so saved ViewManager configs and any pending local state will fall back to the default (off) for this one setting. Harmless for the tester, but re-check the flag before trusting a restored config.
The `useFixedDataShape` rename missed this comment, which still named `optimizeRecordData` as the reason the local hoist-react `paths` entry is committed enabled.

Also dropped the hoist-react PR reference in favour of naming the config and version - the PR number is being renumbered and a comment that has to track it will just go stale again.
Adds the third record-data representation to the tester, so all three can be measured against each other on identical data. Previously only the sparse default and `useFixedDataShape` were reachable.

- New switch on the Store row. Joins the reload-on-toggle set, as it changes how record data objects are produced.
- Mutual exclusions enforced three ways, matching the existing treatment of `reuseRecords` + `retainRaw`: the incompatible switches disable, a reaction clears them, and `createGridModel()` guards again for configs restored from the ViewManager.
- `retainRaw` is surfaced as inert under it - record data *is* the raw object, so it stays reachable however that flag is set.

Valid on this data because the generated rows already arrive in final form: the extra fields are untyped and the base fields are already numbers and strings, so no `Field.parseVal` is required. Note XSS protection is silently inert under it, as nothing is parsed.
@lbwexler lbwexler closed this Aug 1, 2026
@lbwexler

lbwexler commented Aug 1, 2026

Copy link
Copy Markdown
Member

Superceded by version of this merged to develop!

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.

2 participants