From 9a22787f4e4cc9334b2904fd55aa16a24ed133db Mon Sep 17 00:00:00 2001
From: Steven Lambert <2433219+straker@users.noreply.github.com>
Date: Thu, 23 May 2024 09:42:18 -0600
Subject: [PATCH] perf: memoize DqElement (#4452)

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 the `relatedNodes` for each check. Since each each
duplicate id node was also in the `relatedNodes` for every other node,
this caused the single node to [be converted to a
DqElement](https://github.com/dequelabs/axe-core/blob/develop/lib/core/utils/check-helper.js#L46)
_n_ times. This lead to many performance problems, but specifically
calling the `getSelector` of a DqElement would call `outerHTML` 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.

| Before change (in ms)  | After change (in ms) |
| ------------- | ------------- |
| 21,280.1  | 11,841.1  |

<img width="641" alt=""
src="https://github.com/dequelabs/axe-core/assets/2433219/05a456d8-2256-4e51-a0cb-f4a0d1c8c192">

_Flamechart before the change. The majority of the time is spent in
getSelector_

<img width="583" alt=""
src="https://github.com/dequelabs/axe-core/assets/2433219/1360de7c-5a65-448d-a3ee-327004d4c88c">

_Chrome performance timer of getSelector showing it spent 12000ms total
in the function_

<img width="640" alt=""
src="https://github.com/dequelabs/axe-core/assets/2433219/a5225444-0cfc-4ffb-850a-e428076de5e8">

_Flamechart after the change. Time is now spent mostly resolving the
cache which results in no time spent in getSelector_
---
 lib/core/utils/dq-element.js       |  10 +-
 test/core/utils/dq-element.js      | 241 +++++++++++++++--------------
 test/core/utils/node-serializer.js |  15 +-
 3 files changed, 141 insertions(+), 125 deletions(-)

diff --git a/lib/core/utils/dq-element.js b/lib/core/utils/dq-element.js
index 3cf790fe1a..edf3b6d39e 100644
--- a/lib/core/utils/dq-element.js
+++ b/lib/core/utils/dq-element.js
@@ -4,6 +4,7 @@ import getXpath from './get-xpath';
 import getNodeFromTree from './get-node-from-tree';
 import AbstractVirtualNode from '../base/virtual-node/abstract-virtual-node';
 import cache from '../base/cache';
+import memoize from './memoize';
 
 const CACHE_KEY = 'DqElm.RunOptions';
 
@@ -36,7 +37,10 @@ function getSource(element) {
  * @param {Object} options Propagated from axe.run/etc
  * @param {Object} spec Properties to use in place of the element when instantiated on Elements from other frames
  */
-function DqElement(elm, options = null, spec = {}) {
+const DqElement = memoize(function DqElement(elm, options, spec) {
+  options ??= null;
+  spec ??= {};
+
   if (!options) {
     options = cache.get(CACHE_KEY) ?? {};
   }
@@ -82,7 +86,9 @@ function DqElement(elm, options = null, spec = {}) {
   if (!axe._audit.noHtml) {
     this.source = this.spec.source ?? getSource(this._element);
   }
-}
+
+  return this;
+});
 
 DqElement.prototype = {
   /**
diff --git a/test/core/utils/dq-element.js b/test/core/utils/dq-element.js
index 3e6f036f9e..dbef96ea64 100644
--- a/test/core/utils/dq-element.js
+++ b/test/core/utils/dq-element.js
@@ -1,135 +1,140 @@
-describe('DqElement', function () {
-  'use strict';
+describe('DqElement', () => {
+  const DqElement = axe.utils.DqElement;
+  const fixture = document.getElementById('fixture');
+  const fixtureSetup = axe.testUtils.fixtureSetup;
+  const queryFixture = axe.testUtils.queryFixture;
 
-  var DqElement = axe.utils.DqElement;
-  var fixture = document.getElementById('fixture');
-  var fixtureSetup = axe.testUtils.fixtureSetup;
-  var queryFixture = axe.testUtils.queryFixture;
-
-  afterEach(function () {
+  afterEach(() => {
     axe.reset();
   });
 
-  it('should be exposed to utils', function () {
+  it('should be exposed to utils', () => {
     assert.equal(axe.utils.DqElement, DqElement);
   });
 
-  it('should take a virtual node as a parameter and return an object', function () {
-    var vNode = queryFixture('<div id="target"></div>');
-    var result = new DqElement(vNode);
+  it('should take a virtual node as a parameter and return an object', () => {
+    const vNode = queryFixture('<div id="target"></div>');
+    const result = new DqElement(vNode);
     assert.equal(result.element, vNode.actualNode);
   });
 
-  it('should take an actual node as a parameter and return an object', function () {
-    var vNode = queryFixture('<div id="target"></div>');
-    var result = new DqElement(vNode.actualNode);
+  it('should take an actual node as a parameter and return an object', () => {
+    const vNode = queryFixture('<div id="target"></div>');
+    const result = new DqElement(vNode.actualNode);
     assert.equal(result.element, vNode.actualNode);
   });
 
-  describe('element', function () {
-    it('should store reference to the element', function () {
-      var vNode = queryFixture('<div id="target"></div>');
-      var dqEl = new DqElement(vNode);
+  it('should return the same DqElement when instantiated with the same element', () => {
+    const vNode = queryFixture('<div id="target"></div>');
+    const result = new DqElement(vNode);
+    const result2 = new DqElement(vNode);
+    assert.equal(result, result2);
+  });
+
+  describe('element', () => {
+    it('should store reference to the element', () => {
+      const vNode = queryFixture('<div id="target"></div>');
+      const dqEl = new DqElement(vNode);
       assert.equal(dqEl.element, vNode.actualNode);
     });
 
-    it('should not be present in stringified version', function () {
-      var vNode = queryFixture('<div id="target"></div>');
-      var dqEl = new DqElement(vNode);
+    it('should not be present in stringified version', () => {
+      const vNode = queryFixture('<div id="target"></div>');
+      const dqEl = new DqElement(vNode);
       assert.isUndefined(JSON.parse(JSON.stringify(dqEl)).element);
     });
   });
 
-  describe('source', function () {
-    it('should include the outerHTML of the element', function () {
-      var vNode = queryFixture('<div class="bar" id="target">Hello!</div>');
-      var outerHTML = vNode.actualNode.outerHTML;
-      var result = new DqElement(vNode);
+  describe('source', () => {
+    it('should include the outerHTML of the element', () => {
+      const vNode = queryFixture('<div class="bar" id="target">Hello!</div>');
+      const outerHTML = vNode.actualNode.outerHTML;
+      const result = new DqElement(vNode);
       assert.equal(result.source, outerHTML);
     });
 
-    it('should work with SVG elements', function () {
-      var vNode = queryFixture('<svg aria-label="foo" id="target"></svg>');
-      var result = new DqElement(vNode);
+    it('should work with SVG elements', () => {
+      const vNode = queryFixture('<svg aria-label="foo" id="target"></svg>');
+      const result = new DqElement(vNode);
       assert.equal(result.source, vNode.actualNode.outerHTML);
     });
 
-    it('should work with MathML', function () {
-      var vNode = queryFixture(
+    it('should work with MathML', () => {
+      const vNode = queryFixture(
         '<math display="block" id="target">' +
           '<mrow><msup><mi>x</mi><mn>2</mn></msup></mrow>' +
           '</math>'
       );
 
-      var result = new DqElement(vNode);
+      const result = new DqElement(vNode);
       assert.equal(result.source, vNode.actualNode.outerHTML);
     });
 
-    it('should truncate large elements', function () {
-      var div = '<div class="foo" id="target">';
-      for (var i = 0; i < 300; i++) {
+    it('should truncate large elements', () => {
+      let div = '<div class="foo" id="target">';
+      for (let i = 0; i < 300; i++) {
         div += i;
       }
       div += '</div>';
-      var vNode = queryFixture(div);
-      var result = new DqElement(vNode);
+      const vNode = queryFixture(div);
+      const result = new DqElement(vNode);
       assert.equal(result.source, '<div class="foo" id="target">');
     });
 
-    it('should use spec object over passed element', function () {
-      var vNode = queryFixture('<div id="target" class="bar">Hello!</div>');
-      var spec = { source: 'woot' };
-      var result = new DqElement(vNode, {}, spec);
+    it('should use spec object over passed element', () => {
+      const vNode = queryFixture('<div id="target" class="bar">Hello!</div>');
+      const spec = { source: 'woot' };
+      const result = new DqElement(vNode, {}, spec);
       assert.equal(result.source, 'woot');
     });
 
-    it('should return null if audit.noHtml is set', function () {
+    it('should return null if audit.noHtml is set', () => {
       axe.configure({ noHtml: true });
-      var vNode = queryFixture('<div class="bar" id="target">Hello!</div>');
-      var result = new DqElement(vNode);
+      const vNode = queryFixture('<div class="bar" id="target">Hello!</div>');
+      const result = new DqElement(vNode);
       assert.isNull(result.source);
     });
 
-    it('should not use spec object over passed element if audit.noHtml is set', function () {
+    it('should not use spec object over passed element if audit.noHtml is set', () => {
       axe.configure({ noHtml: true });
-      var vNode = queryFixture('<div id="target" class="bar">Hello!</div>');
-      var spec = { source: 'woot' };
-      var result = new DqElement(vNode, {}, spec);
+      const vNode = queryFixture('<div id="target" class="bar">Hello!</div>');
+      const spec = { source: 'woot' };
+      const result = new DqElement(vNode, {}, spec);
       assert.isNull(result.source);
     });
   });
 
-  describe('selector', function () {
-    it('should prefer selector from spec object', function () {
-      var vNode = queryFixture('<div id="target" class="bar">Hello!</div>');
-      var spec = { selector: 'woot' };
-      var result = new DqElement(vNode, {}, spec);
+  describe('selector', () => {
+    it('should prefer selector from spec object', () => {
+      const vNode = queryFixture('<div id="target" class="bar">Hello!</div>');
+      const spec = { selector: 'woot' };
+      const result = new DqElement(vNode, {}, spec);
       assert.equal(result.selector, 'woot');
     });
   });
 
-  describe('ancestry', function () {
-    it('should prefer selector from spec object', function () {
-      var vNode = queryFixture('<div id="target" class="bar">Hello!</div>');
-      var spec = { ancestry: 'woot' };
-      var result = new DqElement(vNode, {}, spec);
+  describe('ancestry', () => {
+    it('should prefer selector from spec object', () => {
+      const vNode = queryFixture('<div id="target" class="bar">Hello!</div>');
+      const spec = { ancestry: 'woot' };
+      const result = new DqElement(vNode, {}, spec);
       assert.equal(result.ancestry, 'woot');
     });
   });
 
-  describe('xpath', function () {
-    it('should prefer selector from spec object', function () {
-      var vNode = queryFixture('<div id="target" class="bar">Hello!</div>');
-      var spec = { xpath: 'woot' };
-      var result = new DqElement(vNode, {}, spec);
+  describe('xpath', () => {
+    it('should prefer selector from spec object', () => {
+      const vNode = queryFixture('<div id="target" class="bar">Hello!</div>');
+      const spec = { xpath: 'woot' };
+      const result = new DqElement(vNode, {}, spec);
       assert.equal(result.xpath, 'woot');
     });
   });
 
-  describe('absolutePaths', function () {
-    it('creates a path all the way to root', function () {
-      var vNode = queryFixture('<div id="target" class="bar">Hello!</div>');
-      var result = new DqElement(vNode, {
+  describe('absolutePaths', () => {
+    it('creates a path all the way to root', () => {
+      const vNode = queryFixture('<div id="target" class="bar">Hello!</div>');
+      const result = new DqElement(vNode, {
         absolutePaths: true
       });
       assert.include(result.selector[0], 'html > ');
@@ -138,31 +143,31 @@ describe('DqElement', function () {
     });
   });
 
-  describe('nodeIndexes', function () {
-    it('is taken from virtualNode', function () {
+  describe('nodeIndexes', () => {
+    it('is taken from virtualNode', () => {
       fixtureSetup('<i></i><b></b><s></s>');
       assert.deepEqual(new DqElement(fixture.children[0]).nodeIndexes, [1]);
       assert.deepEqual(new DqElement(fixture.children[1]).nodeIndexes, [2]);
       assert.deepEqual(new DqElement(fixture.children[2]).nodeIndexes, [3]);
     });
 
-    it('is taken from spec, over virtualNode', function () {
-      var vNode = queryFixture('<div id="target"></div>');
-      var spec = { nodeIndexes: [123, 456] };
-      var dqElm = new DqElement(vNode, {}, spec);
+    it('is taken from spec, over virtualNode', () => {
+      const vNode = queryFixture('<div id="target"></div>');
+      const spec = { nodeIndexes: [123, 456] };
+      const dqElm = new DqElement(vNode, {}, spec);
       assert.deepEqual(dqElm.nodeIndexes, [123, 456]);
     });
 
-    it('is [] when the element is not in the virtual tree.', function () {
-      var div = document.createElement('div');
-      var dqElm = new DqElement(div);
+    it('is [] when the element is not in the virtual tree.', () => {
+      const div = document.createElement('div');
+      const dqElm = new DqElement(div);
       assert.deepEqual(dqElm.nodeIndexes, []);
     });
   });
 
-  describe('toJSON', function () {
-    it('should only stringify selector and source', function () {
-      var spec = {
+  describe('toJSON', () => {
+    it('should only stringify selector and source', () => {
+      const spec = {
         selector: ['foo > bar > joe'],
         source: '<joe aria-required="true">',
         xpath: ['/foo/bar/joe'],
@@ -171,28 +176,28 @@ describe('DqElement', function () {
         fromFrame: false
       };
 
-      var div = document.createElement('div');
-      var result = new DqElement(div, {}, spec);
+      const div = document.createElement('div');
+      const result = new DqElement(div, {}, spec);
       assert.deepEqual(result.toJSON(), spec);
     });
   });
 
-  describe('merging frames', function () {
-    var dqMain, dqIframe;
-    beforeEach(function () {
-      var tree = fixtureSetup(
+  describe('merging frames', () => {
+    let dqMain, dqIframe;
+    beforeEach(() => {
+      const tree = fixtureSetup(
         '<main id="main"></main><iframe id="iframe"></iframe>'
       );
-      var main = axe.utils.querySelectorAll(tree, 'main')[0];
-      var mainSpec = {
+      const main = axe.utils.querySelectorAll(tree, 'main')[0];
+      const mainSpec = {
         selector: ['#main'],
         ancestry: ['html > body > main'],
         xpath: ['/main']
       };
       dqMain = new DqElement(main, {}, mainSpec);
 
-      var iframe = axe.utils.querySelectorAll(tree, 'iframe')[0];
-      var iframeSpec = {
+      const iframe = axe.utils.querySelectorAll(tree, 'iframe')[0];
+      const iframeSpec = {
         selector: ['#iframe'],
         ancestry: ['html > body > iframe'],
         xpath: ['/iframe']
@@ -200,15 +205,15 @@ describe('DqElement', function () {
       dqIframe = new DqElement(iframe, {}, iframeSpec);
     });
 
-    describe('.mergeSpecs', function () {
-      var mainSpec, iframeSpec;
-      beforeEach(function () {
+    describe('.mergeSpecs', () => {
+      let mainSpec, iframeSpec;
+      beforeEach(() => {
         mainSpec = dqMain.toJSON();
         iframeSpec = dqIframe.toJSON();
       });
 
-      it('merges node and frame selectors', function () {
-        var mergedSpec = DqElement.mergeSpecs(mainSpec, iframeSpec);
+      it('merges node and frame selectors', () => {
+        const mergedSpec = DqElement.mergeSpecs(mainSpec, iframeSpec);
         assert.deepEqual(mergedSpec.selector, [
           iframeSpec.selector[0],
           mainSpec.selector[0]
@@ -223,8 +228,8 @@ describe('DqElement', function () {
         ]);
       });
 
-      it('merges nodeIndexes', function () {
-        var mergedSpec = DqElement.mergeSpecs(mainSpec, iframeSpec);
+      it('merges nodeIndexes', () => {
+        const mergedSpec = DqElement.mergeSpecs(mainSpec, iframeSpec);
         assert.deepEqual(mergedSpec.nodeIndexes, [
           iframeSpec.nodeIndexes[0],
           mainSpec.nodeIndexes[0]
@@ -232,48 +237,48 @@ describe('DqElement', function () {
       });
     });
 
-    describe('DqElement.fromFrame', function () {
-      it('returns a new DqElement', function () {
+    describe('DqElement.fromFrame', () => {
+      it('returns a new DqElement', () => {
         assert.instanceOf(DqElement.fromFrame(dqMain, {}, dqIframe), DqElement);
       });
 
-      it('sets options for DqElement', function () {
-        var options = { absolutePaths: true };
-        var dqElm = DqElement.fromFrame(dqMain, options, dqIframe);
+      it('sets options for DqElement', () => {
+        const options = { absolutePaths: true };
+        const dqElm = DqElement.fromFrame(dqMain, options, dqIframe);
         assert.isTrue(dqElm._options.toRoot);
       });
 
-      it('has props as from mergeSpecs', function () {
-        var spec = DqElement.mergeSpecs(dqMain.toJSON(), dqIframe.toJSON());
-        var dqElm = DqElement.fromFrame(dqMain, {}, dqIframe);
+      it('has props as from mergeSpecs', () => {
+        const spec = DqElement.mergeSpecs(dqMain.toJSON(), dqIframe.toJSON());
+        const dqElm = DqElement.fromFrame(dqMain, {}, dqIframe);
         assert.deepEqual(dqElm.toJSON(), spec);
       });
     });
 
-    describe('DqElement.prototype.fromFrame', function () {
-      it('is false when created without a spec', function () {
+    describe('DqElement.prototype.fromFrame', () => {
+      it('is false when created without a spec', () => {
         assert.isFalse(dqMain.fromFrame);
       });
 
-      it('is false when spec is not from a frame', function () {
-        var specMain = dqMain.toJSON();
-        var dqElm = new DqElement(dqMain, {}, specMain);
+      it('is false when spec is not from a frame', () => {
+        const specMain = dqMain.toJSON();
+        const dqElm = new DqElement(dqMain, {}, specMain);
         assert.isFalse(dqElm.fromFrame);
       });
 
-      it('is true when created with a spec', function () {
-        var dqElm = DqElement.fromFrame(dqMain, {}, dqIframe);
+      it('is true when created with a spec', () => {
+        const dqElm = DqElement.fromFrame(dqMain, {}, dqIframe);
         assert.isTrue(dqElm.fromFrame);
       });
     });
   });
 
-  describe('DqElement.setRunOptions', function () {
-    it('sets options for DqElement', function () {
+  describe('DqElement.setRunOptions', () => {
+    it('sets options for DqElement', () => {
       axe.setup();
-      var options = { absolutePaths: true, elementRef: true };
+      const options = { absolutePaths: true, elementRef: true };
       DqElement.setRunOptions(options);
-      var dqElm = new DqElement(document.body);
+      const dqElm = new DqElement(document.body);
 
       const { element, selector } = dqElm.toJSON();
       assert.equal(element, document.body);
@@ -281,12 +286,12 @@ describe('DqElement', function () {
     });
 
     it('is reset by axe.teardown', () => {
-      var options = { absolutePaths: true, elementRef: true };
+      const options = { absolutePaths: true, elementRef: true };
       DqElement.setRunOptions(options);
       axe.teardown();
 
       axe.setup();
-      var dqElm = new DqElement(document.body);
+      const dqElm = new DqElement(document.body);
       const { element, selector } = dqElm.toJSON();
       assert.isUndefined(element);
       assert.equal(selector, 'body');
diff --git a/test/core/utils/node-serializer.js b/test/core/utils/node-serializer.js
index 5d170de025..897d166d0f 100644
--- a/test/core/utils/node-serializer.js
+++ b/test/core/utils/node-serializer.js
@@ -91,14 +91,19 @@ describe('nodeSerializer', () => {
     });
 
     it('skips computing props turned off with runOptions', () => {
-      const dqElm = new DqElement(fixture);
-
       const throws = () => {
         throw new Error('Should not be called');
       };
-      Object.defineProperty(dqElm, 'selector', { get: throws });
-      Object.defineProperty(dqElm, 'ancestry', { get: throws });
-      Object.defineProperty(dqElm, 'xpath', { get: throws });
+
+      const dqElm = new DqElement(
+        fixture,
+        {},
+        {
+          selector: { get: throws },
+          ancestry: { get: throws },
+          xpath: { get: throws }
+        }
+      );
 
       assert.doesNotThrow(() => {
         nodeSerializer.dqElmToSpec(dqElm, {