Skip to content

Commit 7e39874

Browse files
authored
perf(selector): more caching for faster selector creation (#4611)
While doing some perf testing I discovered axe hits a huge performance bottleneck when there are lots of similar elements on the page. There are essentially three problems with that: 1. Selectors aren't cached. An element tested more than once has a selector created more than once. 2. When an element has no unique features, axe creates the same selector and does a QSA with it. If there are lots of those, axe ends up running the same SQA over and over. That can be avoided by caching too 3. When that QSA returns lots of similar elements, axe will refine the selector using parents. Each time it does that it runs .matches on the similar element with the updated selector. With many similar elements, running QSA again is much faster than matching each individually, especially so if the parent's aren't distinct either, so that the QSA cache can be used. I've been using the following code to test this out (with different numbers in rows and cols): ```html <main> <h1>Hello World</h1> <table></table> </main> <script> const table = document.querySelector('table'); for (let row = 0; row < 200; row++) { const tr = document.createElement('tr'); table.appendChild(tr); for (let col = 0; col < 40; col++) { const td = document.createElement('td'); const btn = document.createElement('button'); td.appendChild(btn); tr.appendChild(td); } } </script> ``` ## Perf test results | btns | cur | memo 1 | memo 2 | 50 | 100 | 500 | 600 | 750 | 1000 | 2000 |--------|--------|--------|--------|--------|-------|------|-------|------|-------|------ | 2000 | 2513 | 1511 | 469 | 519 | 192 | 190 | 195 | 190 | 188 | 464 | 2000 | 2585 | 1524 | 470 | 517 | 200 | 190 | 195 | 198 | 203 | 481 | 2000 | 2540 | 1501 | 475 | 525 | 194 | 203 | 190 | 191 | 188 | 468 | 4000 | 10748 | 6183 | 2500 | 2454 | 2452 | 1433 | 838 | 826 | 823 | 850 | 4000 | 10784 | 6199 | 2400 | 2516 | 2452 | 1433 | 839 | 854 | 845 | 819 | 4000 | 10701 | 6203 | 2400 | 2497 | 2442 | 1444 | 849 | 848 | 866 | 838 | 8000 | 43117 | 26398 | 8166 | 10954 | 10895 | 3680 | 2349 | 2220 | 3800 | 2176 | 8000 | 43365 | 26339 | 9747 | 10576 | 10742 | 3775 | 2313 | 2302 | 3826 | 2169 | 8000 | 44899 | 26167 | 9723 | 10615 | 10729 | 3772 | 2235 | 2210 | 3765 | 2238 | 12000 | 100254 | 61300 | 19291 | 20647 | 20623 | 6036 | 6137 | 6146 | 5974 | 6418 | 12000 | 100406 | 60615 | 19543 | 20924 | 20669 | 6122 | 6022 | 6287 | 6094 | 6108 | 12000 | 101401 | 61792 | 19522 | 20662 | 20834 | 5978 | 6170 | 6155 | 6102 | 6347 Numbers are taken from `Audit reporter` with `performanceTimer: true`. - cur: Current situation - memo 1: memoize doc.querySelectorAll - memo 2: memoize doc.querySelectorAll + getSelector - xxx: memoize 2 + re-run QSA if similar elements > XXX ## Memory usage I've tested this approach on 10 different websites and saw no difference in memory usage between this approach and our current approach. ## Performance on real-world websites I tested this against 20 real-world websites and saw no change in performance between them. The scenario where this matters is when there is lots of repetition in a page such as in very large tables. The important part is that improving performance on for this edge case doesn't seem to hurt performance elsewhere.
1 parent dde04a1 commit 7e39874

File tree

5 files changed

+53
-11
lines changed

5 files changed

+53
-11
lines changed

lib/core/constants.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,11 @@ const definitions = [
2727

2828
const constants = {
2929
helpUrlBase: 'https://dequeuniversity.com/rules/',
30+
// Size of a grid square in pixels
3031
gridSize: 200,
32+
// At a certain point, looping over an array of elements and using .match on them
33+
// is slower than just running querySelectorAll again.
34+
selectorSimilarFilterLimit: 700,
3135
results: [],
3236
resultGroups: [],
3337
resultGroupMap: {},

lib/core/public/run-rules.js

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,19 @@ export default function runRules(context, options, resolve, reject) {
5959

6060
// after should only run once, so ensure we are in the top level window
6161
if (context.initiator) {
62+
if (options.performanceTimer) {
63+
performanceTimer.mark('auditAfterStart');
64+
}
6265
results = audit.after(results, options);
63-
66+
if (options.performanceTimer) {
67+
performanceTimer.mark('auditAfterEnd');
68+
performanceTimer.measure(
69+
'audit.after',
70+
'auditAfterStart',
71+
'auditAfterEnd'
72+
);
73+
performanceTimer.logMeasures('audit.after');
74+
}
6475
results.forEach(publishMetaData);
6576
results = results.map(finalizeRuleResult);
6677
}

lib/core/public/run.js

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { getReporter } from './reporter';
22
import normalizeRunParams from './run/normalize-run-params';
33
import { setupGlobals } from './run/globals-setup';
44
import { assert } from '../utils';
5+
import performanceTimer from '../utils/performance-timer';
56

67
const noop = () => {};
78

@@ -33,11 +34,17 @@ export default function run(...args) {
3334

3435
axe._running = true;
3536
if (options.performanceTimer) {
36-
axe.utils.performanceTimer.start();
37+
performanceTimer.start();
3738
}
3839

3940
function handleRunRules(rawResults, teardown) {
4041
const respond = results => {
42+
if (options.performanceTimer) {
43+
performanceTimer.mark('reporterEnd');
44+
performanceTimer.measure('reporter', 'reporterStart', 'reporterEnd');
45+
performanceTimer.logMeasures('reporter');
46+
performanceTimer.end();
47+
}
4148
axe._running = false;
4249
teardown();
4350
try {
@@ -56,11 +63,10 @@ export default function run(...args) {
5663
}
5764
};
5865

59-
if (options.performanceTimer) {
60-
axe.utils.performanceTimer.end();
61-
}
62-
6366
try {
67+
if (options.performanceTimer) {
68+
performanceTimer.mark('reporterStart');
69+
}
6470
createReport(rawResults, options, respond, wrappedReject);
6571
} catch (err) {
6672
wrappedReject(err);
@@ -69,7 +75,7 @@ export default function run(...args) {
6975

7076
function errorRunRules(err) {
7177
if (options.performanceTimer) {
72-
axe.utils.performanceTimer.end();
78+
performanceTimer.end();
7379
}
7480
axe._running = false;
7581
callback(err);

lib/core/utils/get-selector.js

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ import getNodeAttributes from './get-node-attributes';
44
import matchesSelector from './element-matches';
55
import isXHTML from './is-xhtml';
66
import getShadowSelector from './get-shadow-selector';
7+
import memoize from './memoize';
8+
import constants from '../../core/constants';
79

810
const ignoredAttributes = [
911
'class',
@@ -25,6 +27,7 @@ const ignoredAttributes = [
2527
'aria-valuenow',
2628
'xmlns'
2729
];
30+
2831
const MAXATTRIBUTELENGTH = 31;
2932
const attrCharsRegex = /([\\"])/g;
3033
const newlineChars = /(\r\n|\r|\n)/g;
@@ -346,7 +349,6 @@ function getThreeLeastCommonFeatures(elm, selectorData) {
346349
* @param {RootNode} doc The root node of the document or document fragment
347350
* @returns {String} The selector
348351
*/
349-
350352
function generateSelector(elm, options, doc) {
351353
/*eslint no-loop-func:0*/
352354
// TODO: es-modules_selectorData
@@ -373,8 +375,9 @@ function generateSelector(elm, options, doc) {
373375
} else {
374376
selector = features;
375377
}
376-
if (!similar) {
377-
similar = Array.from(doc.querySelectorAll(selector));
378+
// If there are too many similar element running QSA again is faster
379+
if (!similar || similar.length > constants.selectorSimilarFilterLimit) {
380+
similar = findSimilar(doc, selector);
378381
} else {
379382
similar = similar.filter(item => {
380383
return matchesSelector(item, selector);
@@ -398,6 +401,16 @@ function generateSelector(elm, options, doc) {
398401
* @param {Object} optional options
399402
* @returns {String|Array<String>} Unique CSS selector for the node
400403
*/
401-
export default function getSelector(elm, options) {
404+
function getSelector(elm, options) {
402405
return getShadowSelector(generateSelector, elm, options);
403406
}
407+
408+
// Axe can call getSelector more than once for the same element because
409+
// the same element can end up on multiple DqElements.
410+
export default memoize(getSelector);
411+
412+
// Similar elements create similar selectors. If there are lots of similar elements on the page,
413+
// axe ends up needing to run that same selector many times. We can memoize for a huge perf boost.
414+
const findSimilar = memoize((doc, selector) =>
415+
Array.from(doc.querySelectorAll(selector))
416+
);

test/core/constants.js

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,4 +32,12 @@ describe('axe.constants', function () {
3232
it('should have groups for results', function () {
3333
assert.equal(axe.constants.FAIL_GROUP, 'violations');
3434
});
35+
36+
it('should have a gridSize', function () {
37+
assert.equal(axe.constants.gridSize, 200);
38+
});
39+
40+
it('should have a selectorSimilarFilterLimit', function () {
41+
assert.equal(axe.constants.selectorSimilarFilterLimit, 700);
42+
});
3543
});

0 commit comments

Comments
 (0)