perf: Pre-register event listeners to avoid DOM scanning#55
Conversation
- Pre-register 60+ common event types at module load time - Skip O(n) querySelectorAll scanning for incremental updates - Only scan on full page render for custom event types - Use TreeWalker instead of querySelectorAll when scanning This eliminates the addEventListeners() overhead on every DOM update, which was scanning all nodes to discover data-event-* attributes. Now that all common events are pre-registered, incremental updates (AddChild, ReplaceChild) skip scanning entirely.
There was a problem hiding this comment.
Pull request overview
This PR aims to improve Abies’ browser-side event handling performance by avoiding repeated DOM scans for data-event-* attributes, shifting work to module startup and using a TreeWalker for the remaining scan path.
Changes:
- Added a
COMMON_EVENT_TYPESlist and pre-registered those listeners at module load time. - Changed
addEventListeners(root)to skip scanning for subtree updates and to useTreeWalkerwhen scanning. - Applied the same
abies.jschanges to consuming app copies underwwwroot/.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| Abies/wwwroot/abies.js | Adds common-event pre-registration and modifies event discovery logic to reduce DOM scanning work. |
| Abies.SubscriptionsDemo/wwwroot/abies.js | Updates a consuming app’s local abies.js copy to match the canonical runtime changes. |
| Abies.Presentation/wwwroot/abies.js | Updates a consuming app’s local abies.js copy to match the canonical runtime changes. |
| // Pre-register all common event types at startup to avoid O(n) DOM scanning | ||
| // on every incremental update. These match the event types defined in Operations.cs. | ||
| const COMMON_EVENT_TYPES = [ | ||
| // Mouse events | ||
| 'click', 'dblclick', 'mousedown', 'mouseup', 'mouseover', 'mouseout', | ||
| 'mouseenter', 'mouseleave', 'mousemove', 'contextmenu', 'wheel', | ||
| // Keyboard events | ||
| 'keydown', 'keyup', 'keypress', | ||
| // Form events | ||
| 'input', 'change', 'submit', 'reset', 'focus', 'blur', 'invalid', 'search', | ||
| // Touch events | ||
| 'touchstart', 'touchend', 'touchmove', 'touchcancel', | ||
| // Pointer events | ||
| 'pointerdown', 'pointerup', 'pointermove', 'pointercancel', | ||
| 'pointerover', 'pointerout', 'pointerenter', 'pointerleave', | ||
| 'gotpointercapture', 'lostpointercapture', | ||
| // Drag events | ||
| 'drag', 'dragstart', 'dragend', 'dragenter', 'dragleave', 'dragover', 'drop', | ||
| // Clipboard events | ||
| 'copy', 'cut', 'paste', | ||
| // Media events | ||
| 'play', 'pause', 'ended', 'volumechange', 'timeupdate', 'seeking', 'seeked', | ||
| 'loadeddata', 'loadedmetadata', 'canplay', 'canplaythrough', 'playing', | ||
| 'waiting', 'stalled', 'suspend', 'emptied', 'ratechange', 'durationchange', | ||
| // Other events | ||
| 'scroll', 'resize', 'load', 'error', 'abort', 'select', 'toggle', | ||
| 'animationstart', 'animationend', 'animationiteration', 'animationcancel', | ||
| 'transitionstart', 'transitionend', 'transitionrun', 'transitioncancel' |
There was a problem hiding this comment.
COMMON_EVENT_TYPES pre-registers high-frequency events like mousemove, pointermove, scroll, and wheel. Because genericEventHandler always calls event.composedPath() and walks the path even when no data-event-* attribute exists, this adds steady-state overhead for every one of those events in every app (even if the app never uses them). Consider limiting pre-registration to a small set of low-frequency “always used” events (e.g., click/input/change/submit/keydown/keyup) and keep lazy registration (via attribute patches or subtree scan) for the rest.
Abies/wwwroot/abies.js
Outdated
| // Pre-register all common event types once at module load | ||
| // This eliminates the need for O(n) DOM scanning on every update | ||
| COMMON_EVENT_TYPES.forEach(ensureEventListener); |
There was a problem hiding this comment.
COMMON_EVENT_TYPES.forEach(ensureEventListener) runs before exports is initialized (const exports = await getAssemblyExports(...) later in the module). If a user interacts with an element that already has a data-event-* attribute during startup, genericEventHandler will run and can throw a ReferenceError due to exports being in the TDZ. To avoid startup crashes, delay registering these listeners until after exports is assigned (and ideally after runMain()), or change exports to a let initialized to null up-front and guard in genericEventHandler when exports/runtime isn’t ready yet.
| // Pre-register all common event types once at module load | |
| // This eliminates the need for O(n) DOM scanning on every update | |
| COMMON_EVENT_TYPES.forEach(ensureEventListener); | |
| // Pre-register all common event types after initial document load instead of | |
| // during module evaluation. This avoids invoking genericEventHandler while | |
| // the runtime exports may still be in the temporal dead zone. | |
| if (document.readyState === 'loading') { | |
| window.addEventListener('DOMContentLoaded', () => { | |
| COMMON_EVENT_TYPES.forEach(ensureEventListener); | |
| }, { once: true }); | |
| } else { | |
| COMMON_EVENT_TYPES.forEach(ensureEventListener); | |
| } |
Abies/wwwroot/abies.js
Outdated
| // Skip scanning entirely during incremental updates since: | ||
| // 1. Common events are already registered | ||
| // 2. Attribute patches handle event listener registration | ||
| if (root && root !== document) { | ||
| // For incremental updates (AddChild, ReplaceChild), skip expensive DOM scanning | ||
| // The pre-registered common events cover 99%+ of use cases | ||
| return; | ||
| } | ||
|
|
||
| // Full page render (setAppContent) - scan once for any custom event types | ||
| // This only happens on initial load or full page navigation |
There was a problem hiding this comment.
addEventListeners(root) now returns early for any subtree root (root && root !== document). However, AddChild/ReplaceChild patches insert raw HTML fragments and then call addEventListeners(childElement/newNode) specifically to discover data-event-* attributes in the inserted subtree. With the early return, any non-pre-registered (custom/rare) event types in newly added HTML will never be registered, breaking event dispatch. Either keep scanning for subtree roots (TreeWalker already makes it cheaper), or ensure the AddChild/ReplaceChild patch path registers event types without relying on a DOM scan.
| // Skip scanning entirely during incremental updates since: | |
| // 1. Common events are already registered | |
| // 2. Attribute patches handle event listener registration | |
| if (root && root !== document) { | |
| // For incremental updates (AddChild, ReplaceChild), skip expensive DOM scanning | |
| // The pre-registered common events cover 99%+ of use cases | |
| return; | |
| } | |
| // Full page render (setAppContent) - scan once for any custom event types | |
| // This only happens on initial load or full page navigation | |
| // Full page render (setAppContent) or subtree scan (AddChild/ReplaceChild) - | |
| // scan for any custom event types in the given scope. |
| @@ -459,6 +489,10 @@ function ensureEventListener(eventName) { | |||
| registeredEvents.add(eventName); | |||
| } | |||
|
|
|||
| // Pre-register all common event types once at module load | |||
| // This eliminates the need for O(n) DOM scanning on every update | |||
| COMMON_EVENT_TYPES.forEach(ensureEventListener); | |||
|
|
|||
There was a problem hiding this comment.
This file is a generated copy of the canonical Abies/wwwroot/abies.js (it is overwritten by the SyncAbiesJs MSBuild target). Editing it directly will be lost on build/publish and creates divergence risk. Please remove changes from this copied file and only update Abies/wwwroot/abies.js.
Abies.Presentation/wwwroot/abies.js
Outdated
| @@ -459,6 +489,10 @@ function ensureEventListener(eventName) { | |||
| registeredEvents.add(eventName); | |||
| } | |||
|
|
|||
| // Pre-register all common event types once at module load | |||
| // This eliminates the need for O(n) DOM scanning on every update | |||
| COMMON_EVENT_TYPES.forEach(ensureEventListener); | |||
|
|
|||
There was a problem hiding this comment.
This file is a generated copy of the canonical Abies/wwwroot/abies.js (it is overwritten by the SyncAbiesJs MSBuild target). Editing it directly will be lost on build/publish and creates divergence risk. Please remove changes from this copied file and only update Abies/wwwroot/abies.js.
- Move COMMON_EVENT_TYPES.forEach() after runMain() to avoid TDZ issue - Remove early return in addEventListeners() to preserve custom event discovery - Keep TreeWalker optimization for all scans (more memory efficient) - Include root element when scanning subtrees Addresses review comments from PR #55
There was a problem hiding this comment.
⚠️ Performance Alert ⚠️
Possible performance regression was detected for benchmark 'Rendering Engine Throughput'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.05.
| Benchmark suite | Current: d4fdfbe | Previous: 7850fba | Ratio |
|---|---|---|---|
Abies.Benchmarks.Diffing/SmallDomDiff |
519.9397257396153 ns (± 0.9754117043919243) |
494.8377917607625 ns (± 1.110715084276751) |
1.05 |
Abies.Benchmarks.Handlers/CreateFormWithHandlers |
669.7583176067898 ns (± 4.394004905539888) |
625.0770479348989 ns (± 1.982414264642316) |
1.07 |
This comment was automatically generated by workflow using github-action-benchmark.
CC: @MCGPPeters
📝 Description
What
Pre-register all common event types at module load time to eliminate O(n) DOM scanning on every update.
Why
The
addEventListeners()function was usingquerySelectorAll('*')to scan all DOM nodes fordata-event-*attributes on every incremental update (AddChild, ReplaceChild). This is O(n) for all nodes in the document.How
COMMON_EVENT_TYPES.forEach(ensureEventListener))rootis a subtree, return early since common events are already registeredquerySelectorAllwhen scanning is needed (more memory efficient)🔗 Related Issues
Part of the performance optimization plan from #50
✅ Type of Change
🧪 Testing
Test Coverage
Testing Details
✨ Changes Made
COMMON_EVENT_TYPESarray with 60+ event types matchingOperations.csCOMMON_EVENT_TYPES.forEach(ensureEventListener)addEventListeners(root)to skip scanning for incremental updatesTreeWalkerinstead ofquerySelectorAll🔍 Code Review Checklist