RFC: Replace axe.utils.queue with a native Promise

[stephenmathieson]

Expectation: As a developer, I expect the "thenable" objects in axe-core to behave like a Promise.

Actual: The thenables are somewhat Promise-like, but are very different. They do not "chain" as expected, nor can they be awaited.

Motivation: This will make contributing to axe-core easier, and will make @dequelabs/axe-team more efficient. Additionally, using native Promises will enable us to make our code "cleaner" by introducing async/await rather than chaining q.defer() calls together.

---

It's been suggested by @WilcoFiers that using a native Promise will hugely slow down axe.run() (due to Promises being asynchronous). While this may be true (unproven at this point), I'm of the opinion this conversation should still be had.

[stephenmathieson]

I do not think we'll see issues similar to #1172, as deferral via Promise.resolve() takes ~50 μs (on my machine), whereas setTimeout(()=>{}, 0) takes ~1.3 ms.

Proof:

console.time('setTimeout')
setTimeout(() => console.timeEnd('setTimeout'), 0)
// => setTimeout: 1.278076171875ms

console.time('Promise.resolve')
await Promise.resolve()
console.timeEnd('Promise.resolve')
// => Promise.resolve: 0.05322265625ms

I'd like to hear what others think about this before spending any time here tho. @WilcoFiers @dylanb @dequelabs/axe-team what are your thoughts?

[WilcoFiers]

I don't have a problem putting a polyfill into axe-core for Promise. But removing the queue is a different matter. The queue doesn't use setTimeout, it is synchronous, unless you do an async operation in it, in which case it falls back to async. A fairer comparison would be this:

function bar() {}
function foo() { bar() }
console.time('native')
for (i=0; i < 100000; i++) {
  foo()
}
console.timeEnd('native')
//native: 2.23095703125ms

function foo() {return Promise.resolve()}
console.time('Promise.resolve')
for (i=0; i < 100000; i++) {
  await foo()
}
console.timeEnd('Promise.resolve')
// Promise.resolve: 240.371826171875ms

The queue can easily execute 100k times in a single axe.run, much more than that too. That's significant overhead. Adding promises to axe-core was one of the first things I did when I joined Deque. I ended up canning that work because it caused axe to slow down significantly.

The other reason not to do this is because the queue is only ever used in core, and only a hand full of developers work on core. This isn't code that needs to be updated frequently. We need core to be fast, more than we need it to be easy to change.

---

That said, I would like to replace the queue at some point. It's not the highest of priorities, but there are two things that I'd like to be able to do that we can't do today:

• Make axe.run cancellable. This is useful when testing using DOM observers and other frequent-fire axe.run calls
• Allow result streaming as they are returned, rather than return them as a batch. That way we can populate extension UI while axe is still working, so that people don't have to stair at a spinner for half a minute.

Neither of those things can be done with promises. Observerables on the other hand, such as those from RxJS can do this.

[stephenmathieson]

Thanks for your response and thoughts, Wilco.

---

The queue doesn't use setTimeout, it is synchronous, unless you do an async operation in it, in which case it falls back to async.

Right, I'm not suggesting the queue does use setTimeout. Instead, I just wanted to point out we'd have a much smaller performance degradation using a Promises (rather than setTimeout, which we've used in the past to create deferrals).

A fairer comparison would be this:

Right, again, not suggesting asynchronous code is faster than synchronous. It certainly is not.

The queue can easily execute 100k times in a single axe.run, much more than that too.

First, not to say that I don't believe you, but we should prove this (rather than just making a claim). How do you know how often the queue is run? Can you provide analytical data backing this?

Now, assuming that this is correct, and a queue can run 300k times in a single axe.run call, and assuming every function the queue wraps is synchronous and takes exactly 0 time to run, we'll see an overhead of ~15s (based on Promise.resolve() taking 50μs). This hypothetical case would indeed create considerable overhead, but at what cost? Making the axe-core codebase easier to work in, and the team more efficient might be worth it.

The other reason not to do this is because the queue is only ever used in core, and only a hand full of developers work on core.

I'd like to point out that it's very possible only a handful of developers work in "core" because it's hard to do so.

Additionally, nearly half of the core issues since the beginning of time are still open. This tells me the issues are "too hard" to fix quickly. Making progressive improvments (like this one) should help change that.

This isn't code that needs to be updated frequently. We need core to be fast, more than we need it to be easy to change.

It seems core is updated roughly once a week based on Git history.

Additionally, according to previous commits, core receives nearly the same amount of attention as the "rules", and isn't too far behind "checks":

$ git effort --above 5 lib/*
  path                                                                                                         commits    active days

  lib/checks.................................................................................................. 507         243
  lib/rules................................................................................................... 381         192
  lib/core.................................................................................................... 347         205
  lib/commons................................................................................................. 226         156
  lib/outro.stub.............................................................................................. 10          9
  lib/intro.stub.............................................................................................. 9           8
  lib/misc.................................................................................................... 8           5

That said, I would like to replace the queue at some point. It's not the highest of priorities, but there are two things that I'd like to be able to do that we can't do today:

I think both of your suggestions are great ideas, but would like to discuss them separately from this. Do you want to open an issue to discuss them?

[WilcoFiers]

We've been punting on this for two years. I'm going to close this issue. I don't think this is likely to ever change. The time we'd need to invest to make this change is substantial, and even if the "easier on developer" trade-off was worth it, the performance hit we'd get from it wouldn't be worth it.

That's definitely not to say the queue won't ever be replace. But it won't be with promises. Running synchronous code with promises is just wasteful.

[stephenmathieson]

Running synchronous code with promises is just wasteful.

We don't have to do that. Consider the following:

let result = someFunction()
if (result.then) {
  result = await result
}
console.log('the result is', result)