Skip to content

perf(get-element-stack): improve getElementStack performance on large pages#4026

Merged
straker merged 4 commits intodequelabs:developfrom
qazwsxedcrfvtgb1111:perf-get-element-stack
May 30, 2023
Merged

perf(get-element-stack): improve getElementStack performance on large pages#4026
straker merged 4 commits intodequelabs:developfrom
qazwsxedcrfvtgb1111:perf-get-element-stack

Conversation

@qazwsxedcrfvtgb1111
Copy link
Copy Markdown
Contributor

This pr improves the performance of getElementStack on large pages by caching the results of getScroll.
Tested with axe-extension keyboard igt on this page: https://bop.nv.gov/Board/BoardMtgs/
Saves ~200-400 ms on every getElementStack call

@qazwsxedcrfvtgb1111 qazwsxedcrfvtgb1111 requested a review from a team as a code owner May 19, 2023 18:59
@CLAassistant
Copy link
Copy Markdown

CLAassistant commented May 19, 2023

CLA assistant check
All committers have signed the CLA.

Copy link
Copy Markdown
Contributor

@straker straker left a comment

Choose a reason for hiding this comment

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

Thanks for the pr. I can see how caching getScroll would greatly improve performance when the grid is created as we call it for every ancestor in findScrollRegionParent. A better way to do the caching would be to wrap the function using memoize instead of the using the internal cache. You can see an example of that in get-cell-position.

@straker straker dismissed their stale review May 24, 2023 15:57

Resolved

@straker
Copy link
Copy Markdown
Contributor

straker commented May 30, 2023

Thanks. Approved for security

@straker straker merged commit 0e73be0 into dequelabs:develop May 30, 2023
@Amandeque
Copy link
Copy Markdown

Hi @straker Do we have a target Axe-core release and timeline, this PR will be the part of ? A customer is waiting to get this related issue fixed.

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.

4 participants