Perf: Memoize per-element control evaluation (e_memoize_active_controls) - #4
Draft
collinversluis wants to merge 4 commits into
Draft
Perf: Memoize per-element control evaluation (e_memoize_active_controls)#4collinversluis wants to merge 4 commits into
collinversluis wants to merge 4 commits into
Conversation
The editor bootstrap re-evaluates control visibility for every element on every render, repeatedly calling isActiveControl/convertConditionToConditions inside getActiveControls. On heavy pages this is the dominant cost in the editor.min.js / editor-modules.min.js layers. Cache the result of getActiveControls() per BaseSettingsModel instance when called with default arguments, keyed by a monotonic version counter that is bumped on the Backbone change event. The cache is invalidated on any settings mutation, so callers see identical behavior. Gated behind a new e_memoize_active_controls beta experiment (default inactive) so third-party add-ons that mutate widget schemas or settings outside of the Backbone change channel can opt out. Adds two Playwright sanity specs: - editor-bootstrap-performance.test.ts: builds a heavy page, then opens the editor 3x with the experiment off and 3x with it on, asserting identical element rendering and no >15% bootstrap regression. - editor-bootstrap-cpu-profile.test.ts: captures before/after CPU profiles via CDP for offline analysis (reports/bootstrap-experiment-*.cpuprofile). https://claude.ai/code/session_013xhQk6ZEbAUErmzFX9R9mx
…widgets
Two additional savings under the same e_memoize_active_controls experiment
flag, both targeting the cloneObject hot-spot from the original profile
(~1.8s self-time on a 37s editor load):
1. getStyleControls used to do structuredClone( getActiveControls(...) )
on entry, then iterate and replace each control with
jQuery.extend( {}, defaults, control ) before mutating it. The input
map was never structurally modified, so the deep clone was wasted
work — especially now that getActiveControls returns the same cached
map across calls. Skip the structuredClone when the experiment is on.
2. parseDynamicSettings clones the entire settings object on entry
even when the widget has no __dynamic__ values and no repeater
controls (the common case for stock widgets). Add a fast path that
returns self.attributes directly for that case. The three external
callers (views/base.js:959, controls-css-parser.js:42,
repeater-row.js:80) all read the result without mutating it.
Cache the per-model "has any repeater control" check on first use
since the schema is immutable after init.
https://claude.ai/code/session_013xhQk6ZEbAUErmzFX9R9mx
Phase 8 of the perf series. getElementData() ran structuredClone on this.config.elements[elType] for every element instance. On a 200-element page that's 200 deep clones of the same ~10KB element config, most of which are containers/sections that share the exact same source object. The only per-instance mutation was the inner-section title override, which is now applied once to a separate cached object. All known callers READ properties from the returned config (icon, title, tabs_controls, html_wrapper_class, controls) — none mutate, so cross-instance sharing is safe. Two cache slots per elType: __sharedConfig (regular) and __sharedConfigInnerSection (sections with isInner=true). Gated by e_memoize_active_controls.
…t type
Phase 4 of the perf series for the slow-editor-load issue.
Before, every BaseSettingsModel instance ran mergeControlsSettings, which deep-
extends every control via jQuery.extend(true, {}, baseControl, perWidgetOverrides).
For a 200-element page drawn from 30 widget types, that is ~40,000 deep merges
on every editor boot, and is the largest single contributor to the cloneObject
self-time and GC pressure flagged in the perf report (1.8s cloneObject + 2.3s GC).
This change caches the merged-controls map per (widgetType, isInner) on the
shared elementData. All instances of 'heading top-level' now share one merged
map by reference; the per-control deep-extend runs once per widget type instead
of once per instance. The cache is invalidated when addWidgetsCache merges new
data for a widget (e.g. after the lazy AJAX schema fetch).
Audited the only known mutations of the schema map (container.js:248,258 set
control.global.utilized = true) and confirmed nothing reads them — they are
effectively dead writes, so cross-instance sharing is safe.
Gated by the existing e_memoize_active_controls experiment.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Four related caches behind a single experiment flag (
e_memoize_active_controls), targeting thecloneObject+ condition-evaluation hot-spot (~1.8s self-time on a 37s editor load in the perf-fork profile).Commits
getActiveControlsper settings model — cache the result, invalidate on Backbonechangeevent.getStyleControls+ skipcloneObjectinparseDynamicSettingsfor non-dynamic widgets — the inputs are read-only at the call sites, the clones were defensive but unused.All four gated by
e_memoize_active_controls(Performance tag, beta, default OFF).Impact
Profile-attributable: combined ~400-600ms of editor boot CPU on element-heavy pages. The four caches compound — they share inputs, so memoizing one accelerates the others.
Risk profile
The only behaviorally significant change is the
parseDynamicSettingsfast-path: when settings/options/controls are all undefined AND the widget has no__dynamic__keys AND no repeater controls, returnsself.attributesdirectly instead of a clone. The three external callers (views/base.js,controls-css-parser.js,repeater-row.js) all read without mutating — verified in the commit message.Experiment flag preserved as the kill-switch.
Test plan
changefires)parseDynamicSettingsdoes NOT take the fast pathNotes
Cherry-picked from
claude/great-mayer-klkFh(perf-build fork). This is the largest of four planned surgical PRs from the perf branch — see #1, #2, #3 for the smaller ones.Generated by Claude Code