From 98646e5d26cd4dd76d12f4821a49b99cc5465dfa Mon Sep 17 00:00:00 2001 From: Steven Lambert Date: Thu, 11 Apr 2019 04:39:34 -0600 Subject: [PATCH] feat(rule): add more perf timing metrics to rules (#1472) * split gather and filter to own function * fix this in function * default options * add docs for options.performanceTimer * add function to rule prototype --- doc/API.md | 27 +++++----- lib/core/base/rule.js | 77 ++++++++++++++++++++++++++--- lib/core/utils/performance-timer.js | 7 ++- 3 files changed, 90 insertions(+), 21 deletions(-) diff --git a/doc/API.md b/doc/API.md index 543fa7041d..d2d34ff16a 100644 --- a/doc/API.md +++ b/doc/API.md @@ -338,19 +338,20 @@ The options parameter is flexible way to configure how `axe.run` operates. The d Additionally, there are a number or properties that allow configuration of different options: -| Property | Default | Description | -| --------------- | :------ | :-------------------------------------------------------------------------------------------------------------------------------------- | -| `runOnly` | n/a | Limit which rules are executed, based on names or tags | -| `rules` | n/a | Allow customizing a rule's properties (including { enable: false }) | -| `reporter` | `v1` | Which reporter to use (see [Configuration](#api-name-axeconfigure)) | -| `resultTypes` | n/a | Limit which result types are processed and aggregated | -| `xpath` | `false` | Return xpath selectors for elements | -| `absolutePaths` | `false` | Use absolute paths when creating element selectors | -| `iframes` | `true` | Tell axe to run inside iframes | -| `elementRef` | `false` | Return element references in addition to the target | -| `restoreScroll` | `false` | Scrolls elements back to before axe started | -| `frameWaitTime` | `60000` | How long (in milliseconds) axe waits for a response from embedded frames before timing out | -| `preload` | `false` | Any additional assets (eg: cssom) to preload before running rules. [See here for configuration details](#preload-configuration-details) | +| Property | Default | Description | +| ------------------ | :------ | :-------------------------------------------------------------------------------------------------------------------------------------- | +| `runOnly` | n/a | Limit which rules are executed, based on names or tags | +| `rules` | n/a | Allow customizing a rule's properties (including { enable: false }) | +| `reporter` | `v1` | Which reporter to use (see [Configuration](#api-name-axeconfigure)) | +| `resultTypes` | n/a | Limit which result types are processed and aggregated | +| `xpath` | `false` | Return xpath selectors for elements | +| `absolutePaths` | `false` | Use absolute paths when creating element selectors | +| `iframes` | `true` | Tell axe to run inside iframes | +| `elementRef` | `false` | Return element references in addition to the target | +| `restoreScroll` | `false` | Scrolls elements back to before axe started | +| `frameWaitTime` | `60000` | How long (in milliseconds) axe waits for a response from embedded frames before timing out | +| `preload` | `false` | Any additional assets (eg: cssom) to preload before running rules. [See here for configuration details](#preload-configuration-details) | +| `performanceTimer` | `false` | Log rule performance metrics to the console | ###### Options Parameter Examples diff --git a/lib/core/base/rule.js b/lib/core/base/rule.js index ab6dd4b1fc..45aebccc34 100644 --- a/lib/core/base/rule.js +++ b/lib/core/base/rule.js @@ -89,16 +89,48 @@ Rule.prototype.matches = function() { /** * Selects `HTMLElement`s based on configured selector * @param {Context} context The resolved Context object + * @param {Mixed} options Options specific to this rule * @return {Array} All matching `HTMLElement`s */ -Rule.prototype.gather = function(context) { - 'use strict'; +Rule.prototype.gather = function(context, options = {}) { + const markStart = 'mark_gather_start_' + this.id; + const markEnd = 'mark_gather_end_' + this.id; + const markHiddenStart = 'mark_isHidden_start_' + this.id; + const markHiddenEnd = 'mark_isHidden_end_' + this.id; + + if (options.performanceTimer) { + axe.utils.performanceTimer.mark(markStart); + } + var elements = axe.utils.select(this.selector, context); if (this.excludeHidden) { - return elements.filter(function(element) { + if (options.performanceTimer) { + axe.utils.performanceTimer.mark(markHiddenStart); + } + + elements = elements.filter(function(element) { return !axe.utils.isHidden(element.actualNode); }); + + if (options.performanceTimer) { + axe.utils.performanceTimer.mark(markHiddenEnd); + axe.utils.performanceTimer.measure( + 'rule_' + this.id + '#gather_axe.utils.isHidden', + markHiddenStart, + markHiddenEnd + ); + } } + + if (options.performanceTimer) { + axe.utils.performanceTimer.mark(markEnd); + axe.utils.performanceTimer.measure( + 'rule_' + this.id + '#gather', + markStart, + markEnd + ); + } + return elements; }; @@ -152,9 +184,7 @@ Rule.prototype.run = function(context, options, resolve, reject) { try { // Matches throws an error when it lacks support for document methods - nodes = this.gather(context).filter(node => - this.matches(node.actualNode, node, context) - ); + nodes = this.gatherAndMatchNodes(context, options); } catch (error) { // Exit the rule execution if matches fails reject(new SupportError({ cause: error, ruleId: this.id })); @@ -163,6 +193,7 @@ Rule.prototype.run = function(context, options, resolve, reject) { if (options.performanceTimer) { axe.log( + this.id, 'gather (', nodes.length, '):', @@ -214,7 +245,7 @@ Rule.prototype.run = function(context, options, resolve, reject) { axe.utils.performanceTimer.mark(markChecksEnd); axe.utils.performanceTimer.mark(markEnd); axe.utils.performanceTimer.measure( - 'runchecks_' + this.id, + 'rule_' + this.id + '#runchecks', markChecksStart, markChecksEnd ); @@ -225,6 +256,38 @@ Rule.prototype.run = function(context, options, resolve, reject) { q.then(() => resolve(ruleResult)).catch(error => reject(error)); }; +/** + * Selects `HTMLElement`s based on configured selector and filters them based on + * the rules matches function + * @param {Rule} rule The rule to check for after checks + * @param {Context} context The resolved Context object + * @param {Mixed} options Options specific to this rule + * @return {Array} All matching `HTMLElement`s + */ +Rule.prototype.gatherAndMatchNodes = function(context, options) { + const markMatchesStart = 'mark_matches_start_' + this.id; + const markMatchesEnd = 'mark_matches_end_' + this.id; + + let nodes = this.gather(context, options); + + if (options.performanceTimer) { + axe.utils.performanceTimer.mark(markMatchesStart); + } + + nodes = nodes.filter(node => this.matches(node.actualNode, node, context)); + + if (options.performanceTimer) { + axe.utils.performanceTimer.mark(markMatchesEnd); + axe.utils.performanceTimer.measure( + 'rule_' + this.id + '#matches', + markMatchesStart, + markMatchesEnd + ); + } + + return nodes; +}; + /** * Iterates the rule's Checks looking for ones that have an after function * @private diff --git a/lib/core/utils/performance-timer.js b/lib/core/utils/performance-timer.js index a69123d9b1..8cfd68a5de 100644 --- a/lib/core/utils/performance-timer.js +++ b/lib/core/utils/performance-timer.js @@ -90,7 +90,12 @@ utils.performanceTimer = (function() { window.performance && window.performance.getEntriesByType !== undefined ) { - var measures = window.performance.getEntriesByType('measure'); + // only output measures that were started after axe started, otherwise + // we get measures made by the page before axe ran (which is confusing) + var axeStart = window.performance.getEntriesByName('mark_axe_start')[0]; + var measures = window.performance + .getEntriesByType('measure') + .filter(measure => measure.startTime >= axeStart.startTime); for (var i = 0; i < measures.length; ++i) { var req = measures[i]; if (req.name === measureName) {