Skip to content

Comments

Optimise isCommonBins to speed up flaky ILL system test#40886

Draft
MialLewis wants to merge 9 commits intomainfrom
benchmark_common_bins
Draft

Optimise isCommonBins to speed up flaky ILL system test#40886
MialLewis wants to merge 9 commits intomainfrom
benchmark_common_bins

Conversation

@MialLewis
Copy link
Contributor

@MialLewis MialLewis commented Feb 12, 2026

Description of work

ILLDirectGeometryReductionTest is currently flaky on Windows. Whilst investigating this, it was noted that the test takes ~ 1 minute on unix systems and > 15 minutes on Windows. This seems to be a likely candidate for the test unreliability, though this is not certain.

It was discovered that that majority of the extra time is spend in the function isCommonBins, which runs in a heavily looped and parallelised sequence.

To improve this I have

  • Reduced the scope of the mutex. It was initially needed to stop concurrent threads returning a cached version of the isCommonBins flag, whilst the function itself computing an updated value. The setting of the isValid flag has been moved to after the setting of the isCommonBins flag to alleviate this issue. The mutex is now largely in place to prevent duplicated effort checking the bin values.
  • Added early exists to the checking of bin values. It seems that the windows implementation of std::isnan, std::isinf and std::isfinine are bizarrely the root cause.

Windows test: https://builds.mantidproject.org/job/build_branch/1477/

Closes #xxxx.

To test:

See tests pass, including windows system test.


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?

@MialLewis MialLewis force-pushed the benchmark_common_bins branch from ce5393b to f58abb0 Compare February 12, 2026 16:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant