tests: default to more threads for ui-tests

[matthiaskrgr]

Benchmarks (tested on i5-7200U, 2 cores, 4 threads)

master branch:

cargo test // prime caches
cargo --color=always test  70,39s user 21,91s system 180% cpu 51,035 total
cargo --color=always test  70,77s user 22,13s system 180% cpu 51,579 total
cargo --color=always test  70,97s user 22,12s system 180% cpu 51,673 total

cargo --color=always nextest run  78,74s user 22,27s system 220% cpu 45,829 total
cargo --color=always nextest run  78,46s user 21,92s system 224% cpu 44,674 total
cargo --color=always nextest run  78,31s user 22,21s system 228% cpu 43,909 total

Patched (ui_speedup branch):

cargo test // prime cache
cargo --color=always test  97,51s user 32,02s system 288% cpu 44,905 total
cargo --color=always test  99,19s user 31,91s system 276% cpu 47,436 total
cargo --color=always test  98,47s user 31,84s system 284% cpu 45,744 total

cargo --color=always nextest run  102,18s user 30,80s system 350% cpu 37,902 total
cargo --color=always nextest run  99,75s user 29,86s system 350% cpu 36,935 total
cargo --color=always nextest run  100,36s user 29,93s system 351% cpu 37,061 total

changelog: use more threads for running clippys ui-tests for ~10% walltime speedup

[rust-highfive]

r? @llogiq

(rust-highfive has picked a reviewer for you, use r? to override)

[llogiq]

I worry that this may increase the global load on the system to the point where it outweighs any time savings. Our jobs are not alone on the CI machines and someone has to pay for the cycles. Given this, I don't feel confident r+ing this.

@rustbot label +S-needs-discussion

[matthiaskrgr]

Hm, I would expect the host container to have some mechanism to limit the number of threads that get assigned to some arbitrary limit. Otherwise everyone would just make -j 999 in order to get faster builds 😅

I mostly had users testing locally in mind, I only noticed it while running tests locally while glancing at htop and there I still had some headroom in my cpu utilization which is fixed by this change.

[flip1995]

GitHub actions has a limited amount of cores. This may be higher for the Rust org, but I would be surprised if there wasn't a limit at all. You could check how many threads we have available by temporary adding a step to the CI calling nproc.

So I'm not concerned that this will destroy the CI. Though even for the local user, using all available threads by default might be a bad idea. So I would limit the default to just half of the available threads, if we should go for parallel-by-default.

[matthiaskrgr]

For the record, the behaviour of launching $threads amount of jobs instead of $cores amount of jobs is also in line with how cargo does it when launching rustc compiler jobs.

[flip1995]

That's true. I see a speedup of x1.48 on my i7-5820x @ 6 cores / 12 threads. Only using half the threads reduces the speedup to x1.26. Since running all those tests doesn't take that long (~10s with multithreads for me), I think it should be ok to use all available threads by default.

[matthiaskrgr]

oh wow, didn't think it would have that much of an impact with more cores! (also reminds me to get a faster machine 😭)

[flip1995]

I just tried running it on our server with 40 cores (12 were 100% in use, so effectively 28 cores) at work. I only got a speedup of = x1.03 there.

Ah I just realized: compile-test already uses #cores, but this PR forces it to use #threads instead?

[llogiq]

I worry mostly about the effect on other tenants on the system, but that may be misguided.

In any event, 10% wall time will add up, so I'm not exactly opposed to the PR.

[matthiaskrgr]

Ah I just realized: compile-test already uses #cores, but this PR forces it to use #threads instead?

Ah right, num_cpus::get() will return the number of threads of a system (4 in my case)
https://docs.rs/num_cpus/latest/num_cpus/fn.get.html

[llogiq]

As per our last meeting, @bors r+.

[flip1995]

@bors r=llogiq

Is bors awake?

[bors]

📌 Commit a89c795 has been approved by llogiq

[bors]

⌛ Testing commit a89c795 with merge 7f8760a...

[bors]

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: llogiq
Pushing 7f8760a to master...