Skip to content

Comments

Set system test threads default based on CPU count#40785

Draft
thomashampson wants to merge 2 commits intomainfrom
set-new-default-system-test-threads
Draft

Set system test threads default based on CPU count#40785
thomashampson wants to merge 2 commits intomainfrom
set-new-default-system-test-threads

Conversation

@thomashampson
Copy link
Contributor

@thomashampson thomashampson commented Jan 30, 2026

Description of work

In Jenkins, we set the value of BUILD_THREADS_SYSTEM_TESTS per machine to define how many system test to run in parallel. This adds logic to set a sensible default value of 3/16 multiplied by the core count.

To test:

This is running on a github self-hosted runner here:
thomashampson#3
And here is the run with this change in it:
https://github.com/thomashampson/mantid_github_actions_test/actions/runs/21756642425/job/62768649276?pr=3
The runner has 32 cores, check that the number of system test threads is 6. This currently what we set it to for our Jenkins agents for 32-core machines.


Reviewer

Your comments will be used as part of the gatekeeper process. Comment clearly on what you have checked and tested during your review. Provide an audit trail for any changes requested.

As per the review guidelines:

  • Is the code of an acceptable quality? (Code standards/GUI standards)
  • Has a thorough functional test been performed? Do the changes handle unexpected input/situations?
  • Are appropriately scoped unit and/or system tests provided?
  • Do the release notes conform to the guidelines and describe the changes appropriately?
  • Has the relevant (user and developer) documentation been added/updated?
  • If the PR author isn’t in the mantid-developers or mantid-contributors teams, add a review comment rerun ci to authorize/rerun the CI

Gatekeeper

As per the gatekeeping guidelines:

  • Has a thorough first line review been conducted, including functional testing?
  • At a high-level, is the code quality sufficient?
  • Are the base, milestone and labels correct?

@thomashampson thomashampson added the DevOps Issues and pull requests related to DevOps label Feb 6, 2026
@thomashampson thomashampson marked this pull request as ready for review February 6, 2026 20:39
@sf1919 sf1919 added this to the Release 6.16 milestone Feb 9, 2026
@MialLewis MialLewis moved this to Waiting for Review in ISIS core workstream v6.15.0 Feb 9, 2026
@MialLewis MialLewis requested a review from cailafinn February 9, 2026 10:05
@MialLewis MialLewis moved this from Waiting for Review to In Review in ISIS core workstream v6.15.0 Feb 9, 2026
@cailafinn cailafinn self-assigned this Feb 9, 2026
Copy link
Contributor

@cailafinn cailafinn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, can see that it's working for the GH-Actions node. Just a question about the calculation.

BUILD_THREADS_SYSTEM_TESTS=3
# Default to a 3/16 fraction of the build threads, e.g. 8 for a 32-core machine,
# to allow enough RAM for concurrently running system tests.
BUILD_THREADS_SYSTEM_TESTS=$(( BUILD_THREADS * 3 / 16 ))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think we need to make sure this is an integer here? I'm just considering the STFC Cloud might change again and not have a nicely divisible number of cores.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will do integer division, so will take the floor. However, thinking about it more, maybe we should use RAM to determine the system test threads. E.g. our Windows runners currently have 16GB RAM and 24 cores. With the above calculation, that would mean 4 system test threads, but we are currently limiting to 2. What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that's probably more sensible, since RAM is the bottleneck for the problem we're solving by restricting anyway.

@thomashampson thomashampson marked this pull request as draft February 13, 2026 11:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

DevOps Issues and pull requests related to DevOps

Projects

Status: In Review

Development

Successfully merging this pull request may close these issues.

3 participants