Implement pool for managing forks

[novemberborn]

For #78 we'll end up limiting the number of concurrent forks. We should implement a pool of forks that the Api can use to run each test file.

Here's a rough sketch of a possible implementation:

import assert from 'assert'
import fork from './fork'

class Pool extends EventEmitter {
  constructor (limit) {
    this.limit = limit
    this.active = 0
  }

  fork (file, opts) {
    assert(this.active < this.limit)

    this.active++
    const promise = fork(file, opts)
    promise.catch(() => {}).then(() => {
      this.active--
      this.emit('free')
    })
    return promise
  }
}

[novemberborn]

Currently blocked by #696.

[jamestalmage]

I started async-task-pool with this in mind.

[dcousineau]

We're chasing down huge memory usage in our project breaking CircleCI. With a pool we can certainly help clamp down on runaway peak usage (I'm still going to chase down other ideas).

I do notice there is The Blocker™: Would a default behavior of unlimited sized pool maintaining current behavior and adding a flag for limiting the pool with the caveat that test.only won't work be acceptable as an interim (beta) behavior?

If so would extra hands (read: my time) integrating @jamestalmage's async-task-pool be useful or has enough work been started that my time would be wasted effort?

[jamestalmage]

@vdemedes brought up a good point that async-task-pool really is a poor substitute for Bluebird's concurrency controls (promise.map(handler, {concurrency: poolSize})).

Note that would mean creating a disposer. http://bluebirdjs.com/docs/api/disposer.html
Nevermind. Not needed for this. Getting ahead of myself

[jamestalmage]

Sorry for the incomplete answer, got called away.

Would a default behavior of unlimited sized pool maintaining current behavior and adding a flag for limiting the pool with the caveat that test.only won't work be acceptable as an interim (beta) behavior?

Yes.

If so would extra hands (read: my time) integrating @jamestalmage's async-task-pool be useful

Yes, definitely (but use Bluebird's concurrency controls).

However, it's going to take a pretty deep dive into AVA's codebase, and it's a fairly complex issue. We're also going to have strong opinions on how it impacts the programmatic API. I'm not trying to scare you off helping (we would love it, really!). I just want you to know what you're getting into.

[dcousineau]

@jamestalmage I have a bit of time and have been in worse codebases ;) Will find you on the gitter chat at some point, I should have a minimal POC and thoughts to run by today.

[dcousineau]

@jamestalmage So I pulled this minimal proof of concept out of my morning just to verify the approach.

I think I should be able to pull out the final returned promise into two separate logic paths and use the new "pooled" version when a pool size is requested, leaving default behavior in.

The test.only problem will continue to rear its ugly head for those that use it (we don't, or at least not very often) but being able to control runaway process and memory usage on limited CI environments is worth the tradeoff for losing .only in that context for us.

(I haven't pushed any of this to my fork yet because I wanted to get opinions before I moved forward. Currently I'm symlinking the latest master into the node_modules folder of our project and running our test suite as the "real world" example 😬)

[jamestalmage]

Can you just open a PR? It's easier for us to evaluate that way

[dcousineau]

@jamestalmage certainly, I'll flesh it out some more. I'll indicate its also a POC

[bettiolo]

Is this resolved by #791 ?

[novemberborn]

Yes, thank you for noticing @bettiolo 👍