perf(table): Reduce calls to updateStickyColumnStyles #19739
Conversation
googlebot
added
the
cla: yes
src/cdk/table/table.ts
Outdated
| } | ||
|
|
||
| // If there is a data source and row definitions, connect to the data source unless a | ||
| // connection has already been made. | ||
| if (this.dataSource && this._rowDefs.length > 0 && !this._renderChangeSubscription) { | ||
| this._observeRenderChanges(); | ||
| } else if (forced) { |
There was a problem hiding this comment.
This else if requires a bit of extra thinking for me to understand - this should only occur if !(this.dataSource && this._rowDefs.length > 0 && !this._renderChangeSubscription) && forced? Does it need to be coupled to this particular condition, or can we add an additional if` statement that shows exactly what condition we are trying to meet?
There was a problem hiding this comment.
I can add a comment - the else if is because if the table has not rendered any rows yet (the first condition, basically), then this will be taken care of the first time that the table receives row data.
Without this, we'd compute sticky columns and then have to throw that out and do it again when that data arrives.
|
Awesome thanks for catching this optimization - looks good to me, just small nits on readability |
jelbourn
added
lgtm
action: merge
jelbourn
added
G
#19750) * perf(table) Coalesces style updates after style measurements to reduce layout thrashing Exposes the CoalescedStyleScheduler for use by other related components in a table such as column resize. * Add license * Fix column resize tests * Fixed mdc-table tests * jsdoc * more comments * lint * lint * Added _ * lint * -override * update api * prevent resource leaks * api * readonly * remove changes that are part of #19739 * Change to onStable to work around downstream test failures * api update
#19750) * perf(table) Coalesces style updates after style measurements to reduce layout thrashing Exposes the CoalescedStyleScheduler for use by other related components in a table such as column resize. * Add license * Fix column resize tests * Fixed mdc-table tests * jsdoc * more comments * lint * lint * Added _ * lint * -override * update api * prevent resource leaks * api * readonly * remove changes that are part of #19739 * Change to onStable to work around downstream test failures * api update (cherry picked from commit ef8fc4f)
* perf(table): Reduce the number of times sticky column positioning is computed. * Applied suggested readability improvements. (cherry picked from commit f484e96)
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Tested with the dev app, clicking "add table" in the "Table with sticky headers, footers and columns" example.
Before:
100ms scripting + 59ms rendering
After:
75ms scripting + 39ms rendering
The effect is greater in more complex tables. More optimization PRs to come.