Problem/Motivation

See https://git.drupalcode.org/project/drupal/-/jobs/3373087

Drupal\Tests\package_manager\Kernel\SupportedReleaseValidato 10 passes 190s
Drupal\Tests\package_manager\Kernel\ComposerPluginsValidator 33 passes 388s

Also
https://git.drupalcode.org/project/drupal/-/jobs/3373088
Drupal\Tests\package_manager\Kernel\StageBaseTest 42 passes 271s

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3487826

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

catch created an issue. See original summary.

catch’s picture

Issue summary: View changes

catch’s picture

Status: Active » Needs work

This was already flagged as a problem in #3345222: Optimize Composer calls in FixtureManipulator but the issue got won't fixed. I'm taking a different approach to the performance issue here, so might as well leave that issue where it is.

Two approaches we can take:

1. Try to optimize the test itself by reducing the amount of work that needs to be done in setUp/each method.

2. Split the test into multiple classes and rely on gitlab parallelism to reduce overall test run time.

Obviously #1 is preferable but not always easy. Pushed a couple of commits that at least result in a green ComposerPluginsValidatorTest but didn't run the rest of the kernel or functional tests yet.

catch’s picture

Status: Needs work » Needs review

Before:

Drupal\Tests\package_manager\Kernel\SupportedReleaseValidato 10 passes 190s
Drupal\Tests\package_manager\Kernel\ComposerPluginsValidator 33 passes 388s

After

Drupal\Tests\package_manager\Kernel\SupportedReleaseValidato  10 passes  126s                                      
Drupal\Tests\package_manager\Kernel\ComposerPluginsValidator  30 passes  283s

https://git.drupalcode.org/project/drupal/-/pipelines/350984 is green and finished in 7 minutes and 50 seconds, which is still about 2m50s than our fastest test runs, but also a solid couple of minutes faster than test runs against HEAD at the moment.

catch’s picture

I think we also need to split the test up further.

We can move the test methods (renamed to doWhatever and protected) and the data providers to a trait, then we can have 2 or 3 test classes which define test methods calling the doWhatever helpers with the appropriate @dataProvider annotations. That will let us have two 140 second tests instead of one 180 second tests and bring things back down to around 5 minutes hopefully.

Also need to add @group #slow. But there's enough changes here already that it would be good to get reviews.

catch’s picture

Nearly there I think, what the MR does now:

1. composer update --lock is slow. There are a few places in the FixtureManipulator that called it. Instead of doing that, added explicit opt-in calls, and then call those from the tests that actually need it. This saved tens of seconds from the slowest tests.

2. Split the three very slowests tests (4-6 minutes per test class) into 2-3 classes each with a subset of the test methods. This lets run-tests.sh/gitlab parallel runners do their job.

3. Adds an extra kernel test job in the pipeline to spread the load a bit more.

We might need to add @group #slow to some tests to balance them between the jobs better, but that needs some good runs on the faster test runners to know where to apply this, and haven't seen one of those in this issue yet,not blocking.

catch’s picture

Priority: Normal » Major

We seem to be getting slower AWS instances at the moment so it's hard to compare against previous best times which were around 5-7 minutes. However, we seem to be down to around 8-9 minutes from 11-13 minutes which also matches what I'm seeing running the individual package_manager tests locally. Bumping to major since it's looking like 10-20% off each pipeline. There might be more to do, but we could do that in another issue if any of these tests continue to be persisently the slowest.

nicxvan’s picture

Looks good to me. But I think someone from package manager might want to look too.

nicxvan’s picture

I can't tell if those new tests are relevant, if you can rerun them I reviewed the code more closely and I can RTBC as long as the failure are random.

I'm running into the same issue where I cannot rerun them.

catch’s picture

Rebased on 11.x to get a new run, had to re-run a couple of functional tests, but all green now.

Kernel tests are running very slow in general on gitlab, even relative to functional tests - wondering if we have an overall performance regression affecting them, however these are particularly long running tests regardless.

edit: functional tests were also running at about 5 minutes instead of about 3 minutes for most of the jobs, so not specific to kernel tests. Could be the instances, could be a code change.

nicxvan’s picture

Status: Needs review » Reviewed & tested by the community
eiriksm’s picture

Just randomly jumping in here, but from a quick glance it seems `update --lock' (and all other relevant commands) will run with audit enabled? We should probably also pass "--no-audit" or simply disable it altogether (especially for tests): https://getcomposer.org/doc/03-cli.md#composer-no-audit

I believe that could potentially also shave off some CI time

Forgive me if I missed it by glancing on my phone 🤓

  • alexpott committed 3cf29653 on 11.1.x
    Issue #3487826 by catch: package_manager kernel tests are slow
    
    (cherry...

  • alexpott committed fa75872d on 11.x
    Issue #3487826 by catch: package_manager kernel tests are slow
    
alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed fa75872de78 to 11.x and 3cf296532f1 to 11.1.x. Thanks!

@eiriksm let's open an separate issue to have a look into the no audit option.

eiriksm’s picture

catch’s picture

https://git.drupalcode.org/project/drupal/-/pipelines/357428 just finished in 7 minutes 5 seconds so we approaching the old status quo again. package_manager kernel tests are still the slowest kernel tests in that run, let's see where we get to with #3491268: Look into skipping audit of composer operations in package manager tests.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.