perf(table): memoize dataToRender and $params to cut per-cycle render cost (#19537) - #19603
Open
asrita-pyda wants to merge 1 commit into
Open
perf(table): memoize dataToRender and $params to cut per-cycle render cost (#19537)#19603asrita-pyda wants to merge 1 commit into
asrita-pyda wants to merge 1 commit into
Conversation
… cost (primefaces#19537) Table render latency regressed for large datasets. Two hot paths ran redundant work on every change-detection cycle: - The body binding [value]="dataToRender(...)" re-sliced the data array every CD cycle when paginated, re-firing the TableBody value setter (a forced layout reflow for frozen rows) and churning *ngFor. dataToRender now memoizes its result, returning a referentially stable array until its inputs (data ref, length, first, rows, lazy, paginator) change. Non-array args bypass the cache, preserving exact prior behavior. - BaseComponent.$params, read by cx()/sx()/ptm() per element per cycle, walked the parent-instance chain and allocated two objects on every call. It is now computed once per instance and cached; 'instance' stays the live 'this' so style/PT functions still read live state. No public API or change-detection-strategy changes. Adds deterministic referential-stability tests. Validated: table 73, treetable 174, stepper 58, tabs 73 - all green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Author
|
This PR addresses #19537 — it resolves the Table rendering-latency regression described there. See the PR description for the profiling findings (per-cycle |
Author
|
@mehmetcetin01140 @cagataycivici would you mind reviewing when you have a moment? This is a focused, API-compatible performance fix for the Table render regression in #19537 (memoizes |
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
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.
Closes #19537
Defining the issue
Table rendering latency regressed for large datasets (#19537). Profiling the change-detection hot
path surfaced two pieces of redundant work that run on every CD cycle, because both
Tableandthe per-row
TableBodyuseChangeDetectionStrategy.Default:dataToRender()re-allocates the row array every cycle. The body binds[value]="dataToRender(scrollerOptions.rows)"— a method binding. With a paginator it returned afresh
.slice()on every cycle, which re-fired theTableBody.valuesetter (a forced layoutreflow
getOuterHeight()for frozen rows) and handed*ngFora new collection reference tore-diff.
BaseComponent.$paramsis recomputed per element per cycle.cx()/sx()/ptm()(and the$pt/$style/$globalPT/$defaultPTgetters) read$params, which walked the$parentInstancechain via_getHostInstanceand allocated two objects on every call. For arow directive such as
pSelectableRow([class]="cx('selectableRow')") this ran once per row,per cycle.
Fix
dataToRender()(table.ts) now memoizes its result, returning a referentially stable arrayuntil its inputs change. Cache key: resolved data reference, length, effective
first,rows,lazy,paginator. Sort reassigns_value = [...]and filter reassignsfilteredValue, so anydata change flips the reference and invalidates the cache automatically. Non-array arguments
bypass the cache, preserving the original behavior byte-for-byte.
BaseComponent.$params(basecomponent.ts) is computed once per instance and cached. Itsinputs (
$parentInstance,$name,$hostName) are immutable after construction, andinstanceremains the live
this, so style/PT functions still read live state. The cache field is declaredbefore
parent = this.$params.parentso ES2022 define-semantics field init does not reset a cachepopulated during construction.
No public API changes and no change-detection-strategy changes.
Tests
Added deterministic, referential-stability tests (no wall-clock benchmarks) to
table.spec.ts:dataToRender memoization(stable reference, no per-cycle allocation, correct re-slice onfirst/rowschange, sort/filter cache busting, non-array bypass) and$params memoization(stable reference,
instance === component).Validated against the suites that most exercise PassThrough + the
_getHostInstanceparent-chainwalk:
table.spec.ts(incl. 23 PassThrough)treetable.spec.tsstepper.spec.tstabs.spec.tsAll green; no functional regressions in sorting, filtering, pagination, selection, virtual scroll,
templates, or PassThrough.
🤖 Generated with Claude Code