Skip to content

Conversation

@vszakats
Copy link
Member

@vszakats vszakats commented Dec 7, 2025

To improve performance of torture tests.

Also on Windows, where this patch may make those viable for CI.

Linux !FTP 4m47 -> 4m24 (-shallow=25)
Linux FTP 2m30 -> 2m23 (-shallow=25)
macOS !FTP 14m30 -> 13m07 (-shallow=25)
macOS FTP 3m57 -> 3m59 (-shallow=25)
Windows !FTP >25m -> 4m47 to 14m45 (-shallow=5 to 25)

Linux
Before: https://github.com/curl/curl/actions/runs/20006771767/job/57370205514
After: https://github.com/curl/curl/actions/runs/20006783210/job/57370236911?pr=19863

macOS:
Before: https://github.com/curl/curl/actions/runs/20006771786/job/57370205769
After: https://github.com/curl/curl/actions/runs/20006783177/job/57370236995?pr=19863

Windows:
Before: https://github.com/curl/curl/actions/runs/19667198537/job/56326962912?pr=19675
After: https://github.com/curl/curl/actions/runs/20007175773/job/57371768734?pr=19863
After shallow=25: https://github.com/curl/curl/actions/runs/20008523913/job/57374427449?pr=19865

Ref: #19675
Follow-up to 472bc90 #19821

@github-actions github-actions bot added the CI Continuous Integration label Dec 7, 2025
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR optimizes the test runner by converting external memanalyze command calls to direct Perl module calls, reducing process spawning overhead. Performance improvements of 5-10% are demonstrated across Linux and macOS platforms. Additionally, the Windows CI workflow is enhanced with new torture test configurations at various shallow depths.

  • Refactored runner.pm to call memanalyzer as a Perl module instead of spawning external processes
  • Added Windows torture test matrix configurations with different shallow depths (5, 10, 15, 20, 25)
  • Adjusted CI timeout for torture tests from 10 to 25 minutes

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
tests/runner.pm Imports memanalyzer module and converts three backtick command invocations to direct function calls with appropriate parameters
.github/workflows/windows.yml Adds five new torture test matrix entries and increases timeout conditionally for tests with -t flag

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 211 to 213
- { build: 'cmake' , sys: 'mingw64' , env: 'x86_64' , tflags: '-t --shallow=15 !FTP', config: '-DENABLE_DEBUG=ON -DBUILD_SHARED_LIBS=OFF -DCURL_USE_SCHANNEL=ON -DENABLE_UNICODE=ON -DENABLE_ARES=ON', install: 'mingw-w64-x86_64-libssh2', type: 'Debug', name: 'schannel c-ares U torture 15' }
- { build: 'cmake' , sys: 'mingw64' , env: 'x86_64' , tflags: '-t --shallow=10 !FTP', config: '-DENABLE_DEBUG=ON -DBUILD_SHARED_LIBS=OFF -DCURL_USE_SCHANNEL=ON -DENABLE_UNICODE=ON -DENABLE_ARES=ON', install: 'mingw-w64-x86_64-libssh2', type: 'Debug', name: 'schannel c-ares U torture 10' }
- { build: 'cmake' , sys: 'mingw64' , env: 'x86_64' , tflags: '-t --shallow=5 !FTP', config: '-DENABLE_DEBUG=ON -DBUILD_SHARED_LIBS=OFF -DCURL_USE_SCHANNEL=ON -DENABLE_UNICODE=ON -DENABLE_ARES=ON', install: 'mingw-w64-x86_64-libssh2', type: 'Debug', name: 'schannel c-ares U torture 5' }
Copy link

Copilot AI Dec 7, 2025

Choose a reason for hiding this comment

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

The torture test configurations (lines 209-213) enable c-ares with -DENABLE_ARES=ON but don't include mingw-w64-x86_64-c-ares in the install packages. This is inconsistent with line 208 which includes c-ares in both the config and install. Without the c-ares package installed, the -DENABLE_ARES=ON flag may not work correctly or may fail during the build.

Suggested change
- { build: 'cmake' , sys: 'mingw64' , env: 'x86_64' , tflags: '-t --shallow=15 !FTP', config: '-DENABLE_DEBUG=ON -DBUILD_SHARED_LIBS=OFF -DCURL_USE_SCHANNEL=ON -DENABLE_UNICODE=ON -DENABLE_ARES=ON', install: 'mingw-w64-x86_64-libssh2', type: 'Debug', name: 'schannel c-ares U torture 15' }
- { build: 'cmake' , sys: 'mingw64' , env: 'x86_64' , tflags: '-t --shallow=10 !FTP', config: '-DENABLE_DEBUG=ON -DBUILD_SHARED_LIBS=OFF -DCURL_USE_SCHANNEL=ON -DENABLE_UNICODE=ON -DENABLE_ARES=ON', install: 'mingw-w64-x86_64-libssh2', type: 'Debug', name: 'schannel c-ares U torture 10' }
- { build: 'cmake' , sys: 'mingw64' , env: 'x86_64' , tflags: '-t --shallow=5 !FTP', config: '-DENABLE_DEBUG=ON -DBUILD_SHARED_LIBS=OFF -DCURL_USE_SCHANNEL=ON -DENABLE_UNICODE=ON -DENABLE_ARES=ON', install: 'mingw-w64-x86_64-libssh2', type: 'Debug', name: 'schannel c-ares U torture 5' }
- { build: 'cmake' , sys: 'mingw64' , env: 'x86_64' , tflags: '-t --shallow=15 !FTP', config: '-DENABLE_DEBUG=ON -DBUILD_SHARED_LIBS=OFF -DCURL_USE_SCHANNEL=ON -DENABLE_UNICODE=ON -DENABLE_ARES=ON', install: 'mingw-w64-x86_64-libssh2 mingw-w64-x86_64-c-ares', type: 'Debug', name: 'schannel c-ares U torture 15' }
- { build: 'cmake' , sys: 'mingw64' , env: 'x86_64' , tflags: '-t --shallow=10 !FTP', config: '-DENABLE_DEBUG=ON -DBUILD_SHARED_LIBS=OFF -DCURL_USE_SCHANNEL=ON -DENABLE_UNICODE=ON -DENABLE_ARES=ON', install: 'mingw-w64-x86_64-libssh2 mingw-w64-x86_64-c-ares', type: 'Debug', name: 'schannel c-ares U torture 10' }
- { build: 'cmake' , sys: 'mingw64' , env: 'x86_64' , tflags: '-t --shallow=5 !FTP', config: '-DENABLE_DEBUG=ON -DBUILD_SHARED_LIBS=OFF -DCURL_USE_SCHANNEL=ON -DENABLE_UNICODE=ON -DENABLE_ARES=ON', install: 'mingw-w64-x86_64-libssh2 mingw-w64-x86_64-c-ares', type: 'Debug', name: 'schannel c-ares U torture 5' }

Copilot uses AI. Check for mistakes.
Copy link
Member Author

@vszakats vszakats Dec 7, 2025

Choose a reason for hiding this comment

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

Nicely caught. But extending this to Windows will be (if) happening in a different commit; this was only an experiment to see if this patch made Windows torture test speeds more bearable (it did).

@testclutch
Copy link

Analysis of PR #19863 at 596decbc:

Test 2300 failed, which has NOT been flaky recently, so there could be a real issue in this PR. Note that this test has failed in 2 different CI jobs (the link just goes to one of them).

Generated by Testclutch

@vszakats vszakats closed this in f08417c Dec 7, 2025
@vszakats vszakats deleted the runmod branch December 7, 2025 18:52
vszakats added a commit that referenced this pull request Dec 8, 2025
With Schannel and Unicode, `-shallow=13`. It finishes in 12 minutes,
making it the slowest Windows job. It's still on par with torture jobs
on other platforms (though they manage to fit `-shallow=25`).

Also `-shallow=13` still caught leaks in multiple tests.

Also:
- test2300: exclude from CI Windows torture tests.
- experimental.

The downside of going with deeper torture tests, is that it requires
increasing the job timeout. This in turns means that a hung job takes
more minutes to be killed (due to GitHub bugs where a hung step does not
honor the per-step timeout on Windows, another bug where a hung job gets
killed +5 minutes above the workflow timeout, and another bug (or
feature?) where other failed/hung jobs in the the workflow cannot be
restarted till the last job finishes or gets killed. And all this
probably related to a Perl bug which makes it hang on fork errors, which
is turn related to Cygwin/MSYS2 runtime bugs which breaks fork in case
of curl's mixed MSYS2-Perl/native-curl-binaries environment.)
The end result in longer forced waits before being able to restart flaky
jobs, which slows down iterations and annoying.

Also tried:
- non-c-ares job: detected known issues much less often.
- replaced libidn2 with WinIDN: detected known issues much less often.
- runtests -j9-j20 values: did not make a difference.
- other `-shallow` values: 20 is the max feasible, but comes with the
  downside described above.

Ref: #19675 (reboot of)
Follow-up to f08417c #19863

Closes #19865
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI Continuous Integration performance tests

Development

Successfully merging this pull request may close these issues.

3 participants