Problem/Motivation

Soft-postponed on the following issues: #3389510: Split up update semver tests

The reason to postpone this is that without the test refactoring, you don't see the effect on overall run times because individual tests 'book end' the test runs, overrunning all the other tests by up to 3-5 minutes in some cases, this obscures all perceived performance improvements (or regressions) from tweaking the runners.

After the various issues above, we still have several individual functional tests that require 6 minutes to run on gitlab (down from ~13 minutes for InstallUninstallTest and 7-9 minutes for some of the others), but we'll be close to or at diminishing returns with the current strategy at that point. Functional test jobs will finish in 4m30-6m with most tests finishing around the same time vs long overhanging jobs.

This leads to a current best case of 11 minute MR test runs give or take job/runner queuing: https://git.drupalcode.org/project/drupal/-/pipelines/24323

Once we get to that, we can then optimise more tests (probably another 30-60s shaved off is achievable with the splitting strategy for a handful of tests) and tweak more gitlab configuration, but with a much better idea of what the actual effects on performance are.

What this issue does:

1. Moves the functional tests to the top of the child pipeline, this ensures that the slowest tests are queued first. i.e. if we need to queue 15 jobs, we might as well have the 6 minute job queued 30-60s quicker than the 3 minute job. Doesn't affect the UI which gitlab appears to handle alphabetically, although it's hard to tell given our icons.

2. Reduces concurrency to 24, this is because I think we might be hitting CPU contention with 32 concurrency, dropping from 32 to 24 is either a performance improvement or at least performance-neutral. @bbrala has been working on getting numbers, but per above summary the amount of book ending/overruns we have currently makes average CPU usage hard to gauge.

3. Increases functional test parallelism from 6 to 7. This means we go from 6*32=192 to 7 * 24=168 functional tests running at once - as discussed above this results in quicker jobs afaict.

4. Creates 2 parallel jobs each for kernel and functional javascript instead of a single job, this balances out the concurrency lowering (for kernel tests, functional javascript stays at the current core level of 15) and also results in overall faster finish times for those two, which would otherwise be the longest running jobs once functional tests are optimized.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

10 minutes 31 seconds gitlab ci pipeline

(previous runs as blockers got committed)

Screenshot of the gitlab UI with test runs of 14m30s, 13ms0s, 12m28s representing when blocking issues got committed

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#30 Screenshot from 2023-10-02 19-09-20.png87.84 KBcatch
#25 Screenshot from 2023-09-29 16-39-57.png36.34 KBcatch

Issue fork drupal-3388952

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:

About issue forks

Comments

catch created an issue. See original summary.

catch’s picture

catch
he/him
Primary language English
commented
Status: Active » Needs review

catch opened merge request !4844

catch’s picture

catch
he/him
Primary language English
commented
Issue summary: View changes
smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

related issue is RTBC, so marking this so maybe they can merged together.

xjm’s picture

xjm
she/her
Primary language English
commented
Status: Reviewed & tested by the community » Needs work

Merge conflict:

diff --cc core/scripts/run-tests.sh
index 00d32f56a1,cc5b830f41..0000000000
--- a/core/scripts/run-tests.sh
+++ b/core/scripts/run-tests.sh
@@@ -1037,7 -1037,7 +1037,11 @@@ function simpletest_script_get_test_lis
    if ((int) $args['ci-parallel-node-total'] > 1) {
      $slow_tests_per_job = ceil(count($slow_tests) / $args['ci-parallel-node-total']);
      $tests_per_job = ceil(count($test_list) / $args['ci-parallel-node-total']);
++<<<<<<< HEAD
 +    $test_list = array_slice($slow_tests, ($args['ci-parallel-node-index'] -1) * $slow_tests_per_job, $slow_tests_per_job) + array_slice($test_list, ($args['ci-parallel-node-index'] - 1) * $tests_per_job, $tests_per_job);
++=======
+     $test_list = array_merge(array_slice($slow_tests, ($args['ci-parallel-node-index'] -1) * $slow_tests_per_job, $slow_tests_per_job), array_slice($test_list, ($args['ci-parallel-node-index'] - 1) * $tests_per_job, $tests_per_job));
++>>>>>>> 11.x
    }
catch’s picture

catch
he/him
Primary language English
commented
Title: [PP-1] Reduce run-tests.sh concurrency to 16 on gitlab ci » [PP-1] Tweak gitlab CI concurrency, parallelism and test running order
Issue summary: View changes

Going to expand the scope here but also postpone it on the top 3-4 targets from #3389281: [meta] Refactor ultra-slow tests

xjm’s picture

xjm
she/her
Primary language English
commented
Status: Needs work » Postponed

Actually postponing.

catch’s picture

catch
he/him
Primary language English
commented
Title: [PP-1] Tweak gitlab CI concurrency, parallelism and test running order » [PP-5] Tweak gitlab CI concurrency, parallelism and test running order for 11 minute test runs
Priority: Normal » Major
Issue summary: View changes
catch’s picture

catch
he/him
Primary language English
commented
Issue summary: View changes
catch’s picture

catch
he/him
Primary language English
commented
Issue tags: +Test suite performance
catch’s picture

catch
he/him
Primary language English
commented
Issue summary: View changes
catch’s picture

catch
he/him
Primary language English
commented
Title: [PP-5] Tweak gitlab CI concurrency, parallelism and test running order for 11 minute test runs » [PP-2] Tweak gitlab CI concurrency, parallelism and test running order for 11 minute test runs
catch’s picture

catch
he/him
Primary language English
commented
Title: [PP-2] Tweak gitlab CI concurrency, parallelism and test running order for 11 minute test runs » [PP-1] Tweak gitlab CI concurrency, parallelism and test running order for 11 minute test runs
lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Issue summary: View changes

Committed most issues soft-blocking this:

catch’s picture

catch
he/him
Primary language English
commented
Status: Postponed » Needs review

This could be reviewed while the other issue is open since there's no hard dependency, it would just be good to compare a pipeline here once all the other issues are in to the 11.x branch tests to have a real comparison.

catch’s picture

catch
he/him
Primary language English
commented
Issue summary: View changes
catch’s picture

catch
he/him
Primary language English
commented
Title: [PP-1] Tweak gitlab CI concurrency, parallelism and test running order for 11 minute test runs » [PP-2] Tweak gitlab CI concurrency, parallelism and test running order for 11 minute test runs

Missed a child issue, although it's RTBC. Did a rebase last might but gitlab was erroring out due to the docker outage, so going to try luck on the pipeline this morning.

catch’s picture

catch
he/him
Primary language English
commented
Title: [PP-2] Tweak gitlab CI concurrency, parallelism and test running order for 11 minute test runs » [PP-3] Tweak gitlab CI concurrency, parallelism and test running order for 11 minute test runs

Here's the last run: https://git.drupalcode.org/project/drupal/-/pipelines/24975

14m28s but if you click through to the individual jobs for functional and functional javascript tests you can see the ckeditor5, configtranslationui, and updatesemver tests on three out of three of the longest runs. #3389668: Split up ConfigTranslationUiTest wasn't on the list here so back to PP-3.

About to push a commit to slightly reorder the yarn lint jobs, we might be able to squeeze 10-30 seconds from that (start eslint jobs while the other lint jobs are queuing).

catch’s picture

catch
he/him
Primary language English
commented
Issue summary: View changes
catch’s picture

catch
he/him
Primary language English
commented

https://git.drupalcode.org/project/drupal/-/pipelines/25005 13 minutes 2 seconds.

Obviously there's a lot of variation between jobs but looking more like it should. I am pretty confident the remaining three test refactors will take two more minutes off. The best time I've seen on the omnibus MR was 11 minutes 1 second.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Title: [PP-3] Tweak gitlab CI concurrency, parallelism and test running order for 11 minute test runs » [PP-1] Tweak gitlab CI concurrency, parallelism and test running order for 11 minute test runs
Issue summary: View changes

Committed following, back to PP-1:

fjgarlin’s picture

fjgarlin commented
Status: Needs review » Reviewed & tested by the community

Changes look good to me. Mostly adding parallel to some jobs and changing order.

RTBC.

catch’s picture

catch
he/him
Primary language English
commented

Rebased with all-but-one of the blockers committed now.

12 minutes 25 seconds

https://git.drupalcode.org/project/drupal/-/pipelines/25143

catch’s picture

catch
he/him
Primary language English
commented
Issue summary: View changes
StatusFileSize
new Screenshot from 2023-09-29 16-39-57.png36.34 KB
catch’s picture

catch
he/him
Primary language English
commented
Issue summary: View changes
catch’s picture

catch
he/him
Primary language English
commented

Not blockers but potentially another 30 seconds from #3390692: Split out a couple more config translation UI tests and #3390704: Remove/segment duplicate test coverage from ManageFieldsUiTest.

catch’s picture

catch
he/him
Primary language English
commented
Title: [PP-1] Tweak gitlab CI concurrency, parallelism and test running order for 11 minute test runs » Tweak gitlab CI concurrency, parallelism and test running order for 11 minute test runs

#3389510: Split up update semver tests is in! Rebased this to get a fresh run.

catch’s picture

catch
he/him
Primary language English
commented

12m35s but there was a lot of 'waiting for pod' on the lint steps which always leads to variation. https://git.drupalcode.org/project/drupal/-/pipelines/25936

Longest functional test run duration was 6m6s, the two issues linked in #3388952-27: Tweak gitlab CI concurrency, parallelism and test running order for 11 minute test runs will bring that down further but a small incremental changes on top of this.

catch’s picture

catch
he/him
Primary language English
commented
Issue summary: View changes
StatusFileSize
new Screenshot from 2023-10-02 19-09-20.png87.84 KB

Kicked off one more pipeline: https://git.drupalcode.org/project/drupal/-/pipelines/25943 10m31s - fastest one so far, mostly the difference between pod waiting times I think compared to the previous run.

lauriii closed merge request !4844

  • lauriii committed bd32c839 on 11.x 
    Issue #3388952 by catch, fjgarlin: Tweak gitlab CI concurrency,...
lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Status: Reviewed & tested by the community » Fixed

I remember when I started contributing to Drupal core, the test runs were around this long. It's unbelievable that we have been able to match that
with the amount of test coverage we have added since then 🤯

Committed bd32c83 and pushed to 11.x. Thanks!

Status: Fixed » Closed (fixed)

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