Postponed on #3589422: run-tests.sh - separate run progress reporting to a class.
Problem/Motivation
run-tests.sh provides a --concurrency argument to determine the concurrent parallel subprocesses to be executed.
This currently requires a determinate value.
Proposed resolution
Use the fidry/cpu-core-counter to autodetect the number of available CPUs; allow an auto value for the --concurrency argument in run-tests.sh to indicate self detection.
Explicitly require fidry/cpu-core-counter as a dev extension (it's the same package used by PHPStan).
Remaining tasks
User interface changes
Introduced terminology
API changes
Data model changes
Release notes snippet
| Comment | File | Size | Author |
|---|
Issue fork drupal-3526459
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #3
mondrakeMR built on top of #3515347: Reduce run-tests.sh complexity in spawning subprocesses.
Found an interesting bug... no matter what the CONCURRENCY was set on the child pipeline variables, HEAD is anyway using 24 which is what is set on tha main gitlab ci job.
Comment #4
mondrakeComment #5
mondrakeComment #6
mondrakeComment #7
catchHow sure can we be that the CPU detection is returning the CPUs available to the container rather than on the machine?
Comment #8
mondrake#7 honestly I did not investigate that. fidry/cpu-core-counter has a specific method to return the # of CPUs available for parallelisation, and I thought that is exactly what we need for our purposes, so this bit
is calculating the concurrency.
Comment #9
mondrakeComment #11
mondrakeComment #12
mondrakeBlocker is in.
Comment #13
mondrakeThis is worth reviewing/discussing, at this point.
What I saw, is that auto detecting
a) sometimes has a positive effect on wall time, but other times if auto detection is higher than manually entered --concurrency, each individual may take longer, eventually potentially resulting in increased overall wall time. We may think of a parameter to correct the autodetected value.
b) FunctionalJavascript tests suffer high concurrency. Probably here we should just cap the concurrency.
Comment #14
mondrakeActually, this
is the logic that fidry/cpu-core-counter uses to determine the CPUs available for parallelization. So there is already a way to limit the CPU via the $loadLimit parameter, re. 13.a.
Comment #15
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #16
mondrakeComment #17
mondrakeWe could collect the execution log via the event system. Will do that.
Comment #18
mondrakeThis is getting too big. Let's split it up.
Comment #19
fjgarlin commentedHave we considered just checking the
nprocresult and using that for theCONCURRENCYvalue?If it's not in the images, it could be added.
Comment #20
mondrakeI did.
nprocwould just return the number of CPUs. The library call$result = (new CpuCoreCounter())->getAvailableForParallelisation(reservedCpus: 1);, conversely, IMHO does a better job as it also takes into account the CPU load of the server in the last x minutes, therefore limiting the number of processes that will be spawned. And also allows reserving one CPU for the main process. It’s not magic, it’s just a bit of a load balancing approach. Overloading separate jobs on all CPUs would just make all processes run a bit slower.Comment #21
mondrakeSee #14 for the API information.
Comment #22
fjgarlin commentedYup, the library is probably the better approach, as it'll work anywhere it's being run.
My suggestion was just a quick 2-3 lines workaround/fix as I saw that the issue was postponed.
Comment #23
mondrakeYes, let’s make it clear what’s this is postponed on - #3589422: run-tests.sh - separate run progress reporting to a class