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.
Noticed this when trying to debug perf issues in
duplicate-id-aria
. We've run into problems on sites that have a module repeat 1000s of times on the page and the module has an aria id that is also then repeated. Axe-core would take a really long time to run the rule. Looking into it what I discovered is that a majority of the time was spent on resolving therelatedNodes
for each check. Since each each duplicate id node was also in therelatedNodes
for every other node, this caused the single node to be converted to a DqElement n times. This lead to many performance problems, but specifically calling thegetSelector
of a DqElement would callouterHTML
for the node n*2 times which would be very slow.To solve this I decided to memoize the DqElement creation. That way a single node will only ever output a single DqElement, thus saving significant time in creation. Not only that but every time a node appears in a result (either as the check node or a related node) the memory is now shared so this change also reduces the in-memory size of the results object.
Testing a simple page with 5000 nodes of duplicate id, here are the results for running the
duplicate-id-aria
check.Flamechart before the change. The majority of the time is spent in getSelector
Chrome performance timer of getSelector showing it spent 12000ms total in the function
Flamechart after the change. Time is now spent mostly resolving the cache which results in no time spent in getSelector