Skip to content

Commit

Permalink
perf(selector): more caching for faster selector creation (#4611)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
WilcoFiers authored and straker committed Oct 16, 2024
1 parent ae3d6db commit 365acae
Show file tree
Hide file tree
Showing 5 changed files with 53 additions and 11 deletions.
4 changes: 4 additions & 0 deletions lib/core/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,11 @@ const definitions = [

const constants = {
helpUrlBase: 'https://dequeuniversity.com/rules/',
// Size of a grid square in pixels
gridSize: 200,
// At a certain point, looping over an array of elements and using .match on them
// is slower than just running querySelectorAll again.
selectorSimilarFilterLimit: 700,
results: [],
resultGroups: [],
resultGroupMap: {},
Expand Down
13 changes: 12 additions & 1 deletion lib/core/public/run-rules.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,19 @@ export default function runRules(context, options, resolve, reject) {

// after should only run once, so ensure we are in the top level window
if (context.initiator) {
if (options.performanceTimer) {
performanceTimer.mark('auditAfterStart');
}
results = audit.after(results, options);

if (options.performanceTimer) {
performanceTimer.mark('auditAfterEnd');
performanceTimer.measure(
'audit.after',
'auditAfterStart',
'auditAfterEnd'
);
performanceTimer.logMeasures('audit.after');
}
results.forEach(publishMetaData);
results = results.map(finalizeRuleResult);
}
Expand Down
18 changes: 12 additions & 6 deletions lib/core/public/run.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { getReporter } from './reporter';
import normalizeRunParams from './run/normalize-run-params';
import { setupGlobals } from './run/globals-setup';
import { assert } from '../utils';
import performanceTimer from '../utils/performance-timer';

const noop = () => {};

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

axe._running = true;
if (options.performanceTimer) {
axe.utils.performanceTimer.start();
performanceTimer.start();
}

function handleRunRules(rawResults, teardown) {
const respond = results => {
if (options.performanceTimer) {
performanceTimer.mark('reporterEnd');
performanceTimer.measure('reporter', 'reporterStart', 'reporterEnd');
performanceTimer.logMeasures('reporter');
performanceTimer.end();
}
axe._running = false;
teardown();
try {
Expand All @@ -56,11 +63,10 @@ export default function run(...args) {
}
};

if (options.performanceTimer) {
axe.utils.performanceTimer.end();
}

try {
if (options.performanceTimer) {
performanceTimer.mark('reporterStart');
}
createReport(rawResults, options, respond, wrappedReject);
} catch (err) {
wrappedReject(err);
Expand All @@ -69,7 +75,7 @@ export default function run(...args) {

function errorRunRules(err) {
if (options.performanceTimer) {
axe.utils.performanceTimer.end();
performanceTimer.end();
}
axe._running = false;
callback(err);
Expand Down
21 changes: 17 additions & 4 deletions lib/core/utils/get-selector.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import getNodeAttributes from './get-node-attributes';
import matchesSelector from './element-matches';
import isXHTML from './is-xhtml';
import getShadowSelector from './get-shadow-selector';
import memoize from './memoize';
import constants from '../../core/constants';

const ignoredAttributes = [
'class',
Expand All @@ -25,6 +27,7 @@ const ignoredAttributes = [
'aria-valuenow',
'xmlns'
];

const MAXATTRIBUTELENGTH = 31;
const attrCharsRegex = /([\\"])/g;
const newlineChars = /(\r\n|\r|\n)/g;
Expand Down Expand Up @@ -346,7 +349,6 @@ function getThreeLeastCommonFeatures(elm, selectorData) {
* @param {RootNode} doc The root node of the document or document fragment
* @returns {String} The selector
*/

function generateSelector(elm, options, doc) {
/*eslint no-loop-func:0*/
// TODO: es-modules_selectorData
Expand All @@ -373,8 +375,9 @@ function generateSelector(elm, options, doc) {
} else {
selector = features;
}
if (!similar) {
similar = Array.from(doc.querySelectorAll(selector));
// If there are too many similar element running QSA again is faster
if (!similar || similar.length > constants.selectorSimilarFilterLimit) {
similar = findSimilar(doc, selector);
} else {
similar = similar.filter(item => {
return matchesSelector(item, selector);
Expand All @@ -398,6 +401,16 @@ function generateSelector(elm, options, doc) {
* @param {Object} optional options
* @returns {String|Array<String>} Unique CSS selector for the node
*/
export default function getSelector(elm, options) {
function getSelector(elm, options) {
return getShadowSelector(generateSelector, elm, options);
}

// Axe can call getSelector more than once for the same element because
// the same element can end up on multiple DqElements.
export default memoize(getSelector);

// Similar elements create similar selectors. If there are lots of similar elements on the page,
// axe ends up needing to run that same selector many times. We can memoize for a huge perf boost.
const findSimilar = memoize((doc, selector) =>
Array.from(doc.querySelectorAll(selector))
);
8 changes: 8 additions & 0 deletions test/core/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,12 @@ describe('axe.constants', function () {
it('should have groups for results', function () {
assert.equal(axe.constants.FAIL_GROUP, 'violations');
});

it('should have a gridSize', function () {
assert.equal(axe.constants.gridSize, 200);
});

it('should have a selectorSimilarFilterLimit', function () {
assert.equal(axe.constants.selectorSimilarFilterLimit, 700);
});
});

0 comments on commit 365acae

Please sign in to comment.