feat(ByRole): improved byRole query performance #1086
Conversation
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 95e546e:
|
Codecov Report
@@ Coverage Diff @@
## main #1086 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 24 24
Lines 989 990 +1
Branches 322 322
=========================================
+ Hits 989 990 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more |
This comment has been minimized.
This comment has been minimized.
eps1lon
changed the title
There was a problem hiding this comment.
Thanks. Nicely spotted!
Hoisting some of the code up so that it doesn't need to access closures definitely makes sense. Not just from a big-O notation standpoint but from a JS engine perspective as well.
I'm not so sure about the introduce short-circuits though. Could you clarify that?
More minimal change to control flow
|
@all-contributors add @kalmi for code |
|
I've put up a pull request to add @kalmi! 🎉 |
|
🎉 This PR is included in version 8.17.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
What: byRole query logic is optimised for performance
Why: We experienced timeouts while using byRole queries, and decided to investigate by performing profiling. We found that each and every byRole query was taking at least hundred(s) of milliseconds in our test cases (even when accessibility/visibility checks were disabled by mocking out getComputedStyle to always return undefined for any property). Our test cases have medium sized trees, nothing excessive.
How: the byRole query logic is refactored, so that less work done in the tight loop of byRole queries. That loop is executed for each and every node and role combination for every element for every byRole query. Some code is moved out of the loop into the surrounding closure, so that some values are precomputed. The most important change is that a cheap tagName equality check is done, and we skip most of the work if the it can't possibly be a match. These changes were done in a way that we now avoid most allocations in the tightest part of the loop.
These changes result in a 2 orders of magnitude speed up for byRole queries according to my profiling results in out test cases. (This was measured while accessibility/visibility checks were disabled (both before and after) by mocking out getComputedStyle to always return undefined for any property.)
Checklist:
docs site N/A