Skip to content

Cache the column list per table and requester - #206

Merged
NullVoxPopuli merged 1 commit into
universal-ember:mainfrom
johanrd:columns-for-cache
Sep 23, 2026
Merged

NullVoxPopuli merged 1 commit into
universal-ember:mainfrom
johanrd:columns-for-cache

Conversation

@johanrd

@johanrd johanrd commented Sep 23, 2026

Copy link
Copy Markdown
Contributor

columns.for(table) returns a new array on every call, so reading it once per rendered row costs O(rows × columns) per render. Nothing at the call site says so, and a row component that takes @table and reads the list itself has the same cost without looking like it — that is the shape I hit in our app.

This caches the list per table and requester. createCache participates in autotracking, so it recomputes when the columns, their order or their visibility change.

lib=a, 10k rows virtualized, 5 plugins, medians, one build per column:

scenario read before after
reorder, 240 cols per row 126.5 ms 20.6 ms
reorder, 240 cols once 17.7 ms 16.7 ms
reorder, 60 cols per row 12.4 ms 4.7 ms
reorder, 60 cols once 4.7 ms 4.8 ms
identity-swap per row 2.4 ms 2.3 ms
identity-swap once 2.1 ms 2.3 ms

The data path is unaffected. @cached on ColumnReordering's own getters is not equivalent — that moves identity-swap 1.9 → 2.4 ms, which is why this caches at the accessor instead.

test-app: 189 of 189 pass. tsc, prettier and eslint clean.

Cowritten by Claude

`columns.for(table)` walked the plugin chain and returned a new array on
every call, so reading it once per rendered row cost O(rows x columns)
per render. Nothing at the call site said so, and a row component that
takes the table and reads the list itself has the same cost without
looking like it.

`createCache` participates in autotracking, so the list is recomputed
when the columns, their order or their visibility change, and reused
otherwise.

Cowritten by Claude

@NullVoxPopuli NullVoxPopuli left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems reasonable

@NullVoxPopuli
NullVoxPopuli merged commit 94e1a3f into universal-ember:main Sep 23, 2026
15 checks passed
@NullVoxPopuli NullVoxPopuli added the enhancement New feature or request label Sep 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants