Limit concurrency to the number of CPU cores #1467
Conversation
|
I'm so looking forward to this change! |
We should follow |
api.js
Outdated
| @@ -160,16 +161,17 @@ class Api extends EventEmitter { | |||
| this._setupTimeout(runStatus); | |||
| } | |||
|
|
|||
| let overwatch; | |||
| let concurrency = Math.max(2, os.cpus().length || 1); | |||
There was a problem hiding this comment.
Could you add a comment explaining the logic here? Like, why use 2 on single-core machines?
When would os.cpus() return an empty array?
There was a problem hiding this comment.
Could you add a comment explaining the logic here? Like, why use 2 on single-core machines?
Will do. It's to make it still concurrent on VMs that report only one CPU: sindresorhus/grunt-concurrent@ef076ac Does that make sense? It might not, I dunno. Edge-case anyway, so can easily remove it.
When would os.cpus() return an empty array?
Some years ago someone reported an issue about it returning an empty array for a VM or something. I can't find the issue now, so let's just remove it and see.
There was a problem hiding this comment.
Actually, I'm just going to remove it. Too edge-casey and magic.
| @@ -16,6 +16,8 @@ AVA uses [is-ci](https://github.com/watson/is-ci) to decide if it's in a CI envi | |||
|
|
|||
| You may be using a service that only allows a limited number of concurrent connections. For example, many database-as-a-service businesses offer a free plan with a limit on how many clients can be using it at the same time. AVA can hit those limits as it runs multiple processes, but well-written services should emit an error or throttle in those cases. If the one you're using doesn't, the tests will hang. | |||
|
|
|||
| By default, AVA will use as many processes as there are CPU cores in your machine. | |||
readme.md
Outdated
| @@ -400,7 +400,7 @@ test.only('will be run', t => { | |||
| }); | |||
| ``` | |||
|
|
|||
| `.only` applies across all test files, so if you use it in one file, no tests from the other file will run. | |||
| *Note:* The `.only` modifier applies to the test file it's defined in, so if you run multiple test files, tests in other files will stil run. If you want to only run the `test.only` test, specify just that test file to AVA. | |||
There was a problem hiding this comment.
Typo in stil run.
Rather than specify just that test file to AVA, say provide just that test file to AVA?
Agreed → #1472 |
|
@novemberborn Anything else? Travis is failing on |
I'm hoping #1477 fixes that. |
Fixes #966
This considerably improves the performance for the common case. Most users have many test files and their CPUs got overloaded. And few knew about the concurrency option.
This has the downside of making
test.only()not be exclusive with multiple test files, but there are just too many limitations caused by it. I think this change is worth it.I've chosen to go with a default concurrency of the number of CPU cores. We can tweak it based on real usage data later on.