perf: cache getBoundingClientRect in EventHandler to avoid layout thrashing - #823
perf: cache getBoundingClientRect in EventHandler to avoid layout thrashing#823NemeZZiZZ wants to merge 1 commit into
Conversation
|
Are there any important data comparisons before and after optimization? |
|
Benchmark data (measured on current
Instrumented geometry reads during the dirty run (1000 events):
So the cache does eliminate every In other words: this PR is correct and regression-free (clean case unchanged, listener lifecycle verified), but on its own it does not remove the layout thrashing on the crosshair path — Happy to prepare that follow-up as a separate PR if you want the full fix, or to close this one if you would rather solve both in one place. |
Problem
EventHandler._makeCompatEvent(called on everymousemove/touchmove/mousedown) and_startPinchcallthis._target.getBoundingClientRect()onevery event. On hi-DPI trackpads that is 120+ events per second, and
getBoundingClientRect()forces a synchronous layout reflow — the browsermust compute the full layout before returning the rect. This is the classic
layout-thrashing pattern and shows up as jank during pan / zoom / hover.
Fix
Cache the rect in
_boundingRectCacheand invalidate it on the events that canmove or resize the target:
Invalidation triggers:
windowlistener withcapture: true.scrolldoes notbubble, but capture-phase listeners on ancestors fire for descendant scroll
containers, so scrolling any nested container invalidates the cache.
passive: truenever blocks scrolling.
ResizeObserveron_target(covers window resize andlayout-caused target resize).
mousemoveis already re-registered in_mouseEnterHandler).The two
getBoundingClientRect()call sites (_makeCompatEvent,_startPinch)are switched to
_readBoundingRect(). Listeners and the observer are removed indestroy()(the chainChart.destroy→Event.destroy→EventHandler.destroyreaches it), so there is no leak.
isValid(ResizeObserver)is used for the observer guard, matching the existingpattern in
Chart.ts:194.Known limitation
If the target is moved by a CSS transform / margin on an ancestor without a
scroll or resize (pure positional translation), the cache is not invalidated and
the crosshair stays offset by the delta until the next
mouseEnter. This is rarein stable chart layouts and self-corrects on the next hover session — the same
tradeoff used by
lightweight-charts. If it matters for a given use case, aMutationObserveron ancestors could be added (at extra overhead).Verification
_readBoundingRect()only replaces the read;_makeCompatEvent/_startPinchonly read
box.left/box.top(no mutation), so returning the cached object is safe.addEventListener('scroll', invalidate, { passive: true, capture: true })andremoveEventListener('scroll', invalidate, { capture: true })match on thecaptureflag (the only flag
removeEventListenerconsiders for matching), so thelistener is removed cleanly in
destroy().pnpm code-lint— pass (154 files, no fixes)pnpm type-check— passpnpm build-umd:prod— passNotes
EventHandleris instantiated once per chart;_targetis the chart container.page scroll and window resize, pan/zoom smoothness on trackpad) is recommended.