Skip to content

perf: coalesce indicator recalculations on data updates - #835

Open
NemeZZiZZ wants to merge 1 commit into
klinecharts:mainfrom
NemeZZiZZ:perf/coalesce-indicator-recalc
Open

NemeZZiZZ wants to merge 1 commit into
klinecharts:mainfrom
NemeZZiZZ:perf/coalesce-indicator-recalc

Conversation

@NemeZZiZZ

Copy link
Copy Markdown
Contributor

Problem

On every single-bar data update (subscribeBar callback → Store._addData_calcIndicator),
each indicator is recalculated over the entire data list:

// src/Store.ts
private _calcIndicator(data) {
  ...
  indicators.forEach((indicator) => {
    tasks[indicator.id] = indicator.calcImp(this._dataList)
  })
  this._taskScheduler.add(tasks)
}

calc runs synchronously inside calcImp (await this.calc(dataList, this) — the user
callback executes before any await), so a burst of synchronous updates — a websocket catch-up
delivering 100 ticks — performs 100 full recalculations (for VOL: dataList.map(...) over all
bars, allocating a complete new result array each time) in one blocking main-thread task.

Measured on main (production UMD, Chromium, 10 000 bars, candles + VOL, burst of 100
synchronous subscribeBar updates, instrumented _calcIndicator):

metric value
synchronous burst time 286.7 ms
calc invocations 100
time spent inside calc 284.9 ms (99.3% of the burst)
peak heap delta +121.2 MB

This is the library's weakest scenario in a head-to-head benchmark against lightweight-charts
(7.7× slower on a 100-update burst, 3.1× peak heap), while initial render, pan/zoom/hover and
steady-state streaming are at parity or better.

Fix

Coalesce the recalculation into a microtask flush. Marking is immediate; the actual calc
calls happen once per microtask, over the current data list:

indicators.forEach((indicator) => {
  this._pendingCalcIndicators.set(indicator.id, indicator)
})
if (!this._calcFlushScheduled) {
  this._calcFlushScheduled = true
  void Promise.resolve().then(() => {
    this._calcFlushScheduled = false
    ...
    this._pendingCalcIndicators.forEach((indicator) => {
      tasks[indicator.id] = indicator.calcImp(this._dataList)
    })
    this._pendingCalcIndicators.clear()
    this._taskScheduler.add(tasks)
  })
}

This is safe because calc is a pure function of the data list (plus the indicator itself):
one flush after the last update of a synchronous burst produces the same result as
recalculating after every intermediate update — the intermediate results were never rendered.
The TaskScheduler and its layout callback are unchanged, so the flush → recalculate → layout
ordering is preserved (one microtask later than before).

State is cleared in destroy().

Result (same instrumented setup)

metric main this PR
synchronous burst time (100 updates) 286.7 ms 1.8 ms
calc invocations 100 1
peak heap delta +121.2 MB +10.3 MB

Verification

  • pnpm code-lint — pass (154 files, no fixes)
  • pnpm type-check — pass
  • pnpm build-esm — pass
  • Manual: single updates and initial loads render identically; a 100-update burst no longer
    produces a long task; steady-state 10–20 Hz streaming unchanged.

Notes

  • A single update now recalculates one microtask later than before — imperceptible and already
    asynchronous downstream (TaskScheduler → layout).
  • If an indicator is removed between marking and the flush, its pending entry is dropped
    harmlessly (the flush skips empty pending state).
  • Custom indicators relying on being called once per update (e.g. building side-effect state)
    would see one call per microtask instead — calc is documented as a pure calculation of the
    data list, and render-consumed state is exactly what this preserves.

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