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

Issue fork drupal-3526459

Command icon 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

mondrake created an issue. See original summary.

mondrake’s picture

Title: Autodetect available CPUs in run-tests.sh for concurrency » [PP-1] Autodetect available CPUs in run-tests.sh for concurrency
Status: Active » Postponed

MR 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.

mondrake’s picture

Title: [PP-1] Autodetect available CPUs in run-tests.sh for concurrency » [PP] Autodetect available CPUs in run-tests.sh for concurrency
Issue summary: View changes
Related issues: +#3536964: run-tests.sh - segregate command line parsing and use Symfony Console classes
mondrake’s picture

Issue tags: +run-tests.sh
mondrake’s picture

Issue tags: -PHPUnit 12
catch’s picture

How sure can we be that the CPU detection is returning the CPUs available to the container rather than on the machine?

mondrake’s picture

#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

  // Get the number of available CPUs, we try to reserve 1 for the main
  // process. That will do for total cores above 4; otherwise we use the main
  // CPU too.
  try {
    $result = (new CpuCoreCounter())->getAvailableForParallelisation(reservedCpus: 1);
    return $result->totalCoresCount > 4 ? $result->availableCpus : $result->totalCoresCount;
  } catch (NumberOfCpuCoreNotFound) {
    return 1;
  }

is calculating the concurrency.

mondrake’s picture

Issue tags: +Test suite performance

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.

mondrake’s picture

Issue tags: -PHPUnit 11 +PHPUnit 12
mondrake’s picture

Title: [PP] Autodetect available CPUs in run-tests.sh for concurrency » Autodetect available CPUs in run-tests.sh for concurrency
Issue summary: View changes
Status: Postponed » Active

Blocker is in.

mondrake’s picture

Status: Active » Needs review

This 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.

mondrake’s picture

Actually, this

    /**
     * @param positive-int|0    $reservedCpus      
     *    Number of CPUs to reserve. This is useful when you want
     *    to reserve some CPUs for other processes. If the main
     *    process is going to be busy still, you may want to set
     *    this value to 1.
     * @param non-zero-int|null $countLimit        
     *    The maximum number of CPUs to return. If not provided, it
     *    may look for a limit in the environment variables, e.g.
     *    KUBERNETES_CPU_LIMIT. If negative, the limit will be
     *    the total number of cores found minus the absolute value.
     *    For instance if the system has 10 cores and countLimit=-2,
     *    then the effective limit considered will be 8.
     * @param float|null        $loadLimit         
     *    Element of [0., 1.]. Percentage representing the
     *    amount of cores that should be used among the available
     *    resources. For instance, if set to 0.7, it will use 70%
     *    of the available cores, i.e. if 1 core is reserved, 11
     *    cores are available and 5 are busy, it will use 70%
     *    of (11-1-5)=5 cores, so 3 cores. Set this parameter to null
     *    to skip this check. Beware that 1 does not mean "no limit",
     *    but 100% of the _available_ resources, i.e. with the
     *    previous example, it will return 5 cores. How busy is
     *    the system is determined by the system load average
     *    (see $systemLoadAverage).
     * @param float|null        $systemLoadAverage 
     *    The system load average. If passed, it will use
     *    this information to limit the available cores based
     *    on the _available_ resources. For instance, if there
     *    is 10 cores but 3 are busy, then only 7 cores will
     *    be considered for further calculation. If set to
     *    `null`, it will use `sys_getloadavg()` to check the
     *    load of the system in the past minute. You can
     *    otherwise pass an arbitrary value. Should be a
     *    positive float.
     *
     * @see https://php.net/manual/en/function.sys-getloadavg.php
     */
    public function getAvailableForParallelisation(

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.

needs-review-queue-bot’s picture

Status: Needs review » Needs work
StatusFileSize
new91 bytes

The 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.

mondrake’s picture

Status: Needs work » Needs review
mondrake’s picture

Assigned: Unassigned » mondrake
Status: Needs review » Needs work

We could collect the execution log via the event system. Will do that.

mondrake’s picture

Status: Needs work » Postponed

This is getting too big. Let's split it up.

fjgarlin’s picture

Have we considered just checking the nproc result and using that for the CONCURRENCY value?

- ...
- export CONCURRENCY=$(nproc)
- time sudo -u www-data -E -H php ./core/scripts/run-tests.sh --phpunit-configuration $PHPUNIT_CONFIGURATION_FILE_PATH --debug-discovery --debug-execution --color --keep-results --types "$TESTSUITE" $CONCURRENCY --sqlite ":memory:" --dburl $SIMPLETEST_DB --url $SIMPLETEST_BASE_URL --verbose --non-html --all --ci-parallel-node-index $CI_NODE_INDEX --ci-parallel-node-total $CI_NODE_TOTAL

If it's not in the images, it could be added.

mondrake’s picture

I did. nproc would 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.

mondrake’s picture

See #14 for the API information.

fjgarlin’s picture

Yup, 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.

mondrake’s picture

Title: Autodetect available CPUs in run-tests.sh for concurrency » [PP-1] Autodetect available CPUs in run-tests.sh for concurrency
Issue summary: View changes

Yes, let’s make it clear what’s this is postponed on - #3589422: run-tests.sh - separate run progress reporting to a class