perf: defer rules rather than checks #1308
Conversation
This patch removes `setTimeout()` call from `Check#run()` and adds it to `Rule#run()`. The `setTimeout()` was originally added in 4657dc5 in order to prevent browsers from "freezing up" while running 1000s of synchronous checks. This had the downside of creating ~10k timers which slows down `axe.run()` considerably. Moving the `setTimeout()` to `Rule#run()` ensures we continue to run asynchronously to prevent "unresponsive script" warnings, but we defer as little as possible. With this patch, we should see ~40 timers created rather than 10k+ since we now create one per Rule rather than one per Check. Hat tip to @paulirish for starting this conversation in #1172.
|
thanks! This looks perfect to me. 👍 |
|
@stephenmathieson can you confirm you've tested this solution in the Chrome and Firefox extensions? Can you give some numbers on how much this improves performance? |
|
Yes, I've manually tested this in both extensions. I can put together some quick performance results today. |
|
To benchmark, I built axe-core for each both 4489965 and 80c1c74 ( Once built, console.time('axe')
await axe.run()
console.timeEnd('axe')benchmarks from google.comat revision 80c1c74 (where this commit landed):
199.033935546875 + 171.033203125 + 169.7431640625 + 165.174072265625 = 704.984375 at revision 4489965 (prior to this commit):
240.59912109375 + 179.072021484375 + 176.920166015625 + 175.220947265625 = 771.812255859 results With this patch, it takes ~176ms for benchmarks from https://www.guidetoonlineschools.com/online-schoolsNOTE: this is the URL @paulirish originally shared. at revision 80c1c74 (where this commit landed):
22537.302001953125 + 21808.623046875 + 23041.050048828125 + 23189.9990234375 = 90576.9741211 at revision 4489965 (prior to this commit):
24342.35107421875 + 25822.697265625 + 24546.11181640625 + 23406.01513671875 = 98117.175293 results With this patch, it takes ~22.6s to for |
This patch removes
setTimeout()call fromCheck#run()and adds it toRule#run(). ThesetTimeout()was originally added in 4657dc5 in order to prevent browsers from "freezing up" while running 1000s of synchronous checks. This had the downside of creating ~10k timers which slows downaxe.run()considerably.Moving the
setTimeout()toRule#run()ensures we continue to run asynchronously to prevent "unresponsive script" warnings, but we defer as little as possible. With this patch, we should see ~40 timers created rather than 10k+ since we now create one per Rule rather than one per Check.Hat tip to @paulirish for starting this conversation in #1172.
Reviewer checks
Required fields, to be filled out by PR reviewer(s)