Improve runner performance

[jamestalmage]

The main premise I had while creating this: Deferring when you don't need to is expensive.

This rewrites the runner and test to use the following semantics:

If a test never uses an async features (i.e. cb mode, t.throws(promise), return a promise / observable) the next test is executed synchronously.

This requires a unified API between tests and the runner:

// returns a "result" or promise for a result
var result = runnable.run();

// a "result" has the following properties:
result = {
  passed: true || false,
  reason: Error,
  result: "some result"
}

Because run can return either a result or a promise for a result, you frequently need to check which it is using is-promise. This leads to some verbose code, but it runs faster, and is fairly well encapsulated in the sequence and concurrent classes.

AVA master:

ava --verbose  9.38s user 0.63s system 461% cpu 2.173 total

This PR:

ava --verbose  8.46s user 0.65s system 522% cpu 1.744 total

[jamestalmage]

All working now, using benchmarks from jamestalmage/emoji-aware#no-assertion-plans:

Master:

ava --verbose  6.86s user 0.56s system 439% cpu 1.689 total
ava --verbose  6.78s user 0.55s system 438% cpu 1.669 total
ava --verbose  6.73s user 0.55s system 432% cpu 1.681 total

This PR:

ava --verbose  5.25s user 0.53s system 432% cpu 1.337 total
ava --verbose  4.97s user 0.50s system 419% cpu 1.303 total
ava --verbose  5.05s user 0.50s system 426% cpu 1.301 total

[jamestalmage]

Using the following benchmark (no assertions - just tracking runner performance):

import test from './';

for (var i = 0; i < 10000; i++) {
    test('test' + i, t => {});
}

Master:

node ./cli.js benchmark.js --verbose  3.15s user 0.31s system 104% cpu 3.318 total
node ./cli.js benchmark.js --verbose  2.76s user 0.28s system 103% cpu 2.937 total
node ./cli.js benchmark.js --verbose  2.73s user 0.28s system 103% cpu 2.911 total

This PR:

node ./cli.js benchmark.js --verbose  1.44s user 0.25s system 107% cpu 1.573 total
node ./cli.js benchmark.js --verbose  1.40s user 0.25s system 106% cpu 1.544 total
node ./cli.js benchmark.js --verbose  1.38s user 0.24s system 108% cpu 1.499 total

[jamestalmage]

This also seems to provide a slight performance boost for async tests:

import test from './';

for (var i = 0; i < 10000; i++) {
    test('test' + i, t => {return Promise.resolve()});
}

Master:

node ./cli.js benchmark.js --verbose  2.84s user 0.28s system 103% cpu 3.010 total
node ./cli.js benchmark.js --verbose  2.86s user 0.28s system 103% cpu 3.033 total
node ./cli.js benchmark.js --verbose  2.82s user 0.28s system 104% cpu 2.980 total

This PR:

node ./cli.js benchmark.js --verbose  2.68s user 0.28s system 103% cpu 2.843 total
node ./cli.js benchmark.js --verbose  2.69s user 0.28s system 104% cpu 2.854 total
node ./cli.js benchmark.js --verbose  2.72s user 0.27s system 104% cpu 2.880 total

[jamestalmage]

Mixed sync/async:

import test from './';

test.beforeEach(t => {});
test.afterEach(t => {});

for (var i = 0; i < 10000; i++) {
    test('test' + i, t => {return Promise.resolve()});
}

Master:

node ./cli.js benchmark.js --verbose  4.78s user 0.43s system 102% cpu 5.075 total
node ./cli.js benchmark.js --verbose  4.64s user 0.42s system 102% cpu 4.953 total
node ./cli.js benchmark.js --verbose  4.75s user 0.43s system 102% cpu 5.041 total

This PR:

node ./cli.js benchmark.js --verbose  3.56s user 0.38s system 103% cpu 3.797 total
node ./cli.js benchmark.js --verbose  3.56s user 0.38s system 103% cpu 3.791 total
node ./cli.js benchmark.js --verbose  3.62s user 0.39s system 103% cpu 3.888 total

[sindresorhus]

Should be TypeError to match ES2015 classes and message: Class constructor Concurrent cannot be invoked without 'new'

[sindresorhus]

👏 From a quick skim this looks very promising, although a lot to take in. I like the improved separation of concerns and performance improvements, but not totally happy about the added verboseness. Would be nice if we could follow up later with ways to simplify it. I'll try to do a more thorough review tomorrow.

@vdemedes @ariporad @sotojuan @novemberborn I could use some help reviewing this :)

Tips for reviewing: Review the commits individually and use https://github.com/sindresorhus/ava/pull/466/files?w=1 to review the complete diff ignoring whitespace changes (for a more readable diff).

[jamestalmage]

Would be nice if we could follow up later with ways to simplify it.

Agreed. Getting a good benchmarking suite (#460) would be really helpful as well.

[vadimdemedes]

Massive applause for huge amount of work, @jamestalmage.

I have some style/cleanup-related suggestions, but I can follow up with a PR myself ;)

[sindresorhus]

I have some style/cleanup-related suggestions, but I can follow up with a PR myself ;)

Yes, let's do style stuff later. (I did a few myself, but realized it's better not to dilute the review with nitpicks).

[jamestalmage]

Once merged, we should all dogfood this on existing projects before release, (maybe deploy with a next tag for a while as well). There are enough changes here that our regression exposure is pretty large. I've added pretty exhaustive tests, but we should still thoroughly vet it.

[sindresorhus]

Once merged, we should all dogfood this on existing projects before release

👍 I've already tried it on got and pageres and seems to work good.

maybe deploy with a next tag for a while as well

👍

[sotojuan]

Currently in class but from what I skimmed it looks good :-) Awesome work!

[sindresorhus]

Landed! Awesome work @jamestalmage :)

[sotojuan]