Skip to content

[tests] Run swift-(corelibs-)foundation tests in debug configuration on Windows - #80076

Merged
ahoppen merged 1 commit into
swiftlang:mainfrom
ahoppen:foundation-tests-debug
Mar 19, 2025
Merged

ahoppen merged 1 commit into
swiftlang:mainfrom
ahoppen:foundation-tests-debug

Conversation

@ahoppen

@ahoppen ahoppen commented Mar 18, 2025

Copy link
Copy Markdown
Member

This should improve CI times because building in debug configuration instead of release configuration is significantly faster. Since we don’t install the build of swift-(corelibs-)foundation using SwiftPM into the toolchain, this doesn’t have any performance impact on users of the toolchain.

@ahoppen
ahoppen requested a review from jmschonfeld March 18, 2025 00:05
@ahoppen

ahoppen commented Mar 18, 2025

Copy link
Copy Markdown
Member Author

@swift-ci Please smoke test

@compnerd

compnerd commented Mar 18, 2025

Copy link
Copy Markdown
Member

One thing that I did notice (and why we ran the tests in release mode) - the release mode picked up more issues than the debug builds. The other thing I noticed was running tests sequentially uncovered even more issues than running them in parallel (which did one case per process). I wonder if we should do release mode tests in nightlies and debug mode in PR? Likewise, we could run the tests in parallel mode in PR testing, and sequential in nightlies.

@ahoppen

ahoppen commented Mar 18, 2025

Copy link
Copy Markdown
Member Author

@swift-ci Please smoke test macOS

@ahoppen

ahoppen commented Mar 18, 2025

Copy link
Copy Markdown
Member Author

Just got the timing results from the CI runs:

  • swift-foundation test time reduced from 23:35 min to 7:08 min
  • swift-corelibs-foundation test time reduced from 26:24 min to 7:17 min

Total saving: ~35 minutes

@ahoppen

ahoppen commented Mar 18, 2025

Copy link
Copy Markdown
Member Author

the release mode picked up more issues than the debug builds

Interesting. What kind of issues were these? Race conditions that weren’t hit in debug builds?

The other thing I noticed was running tests sequentially uncovered even more issues than running them in parallel

Since running the tests only takes ~1 minute, I don’t think we gain much by running them in parallel anyway. Incidentally, I found that running swift-format tests (also ~1 minute) is slightly faster in serial mode than in parallel mode. Probably because there’s less process launching overhead involved. So, I would keep tests in serial mode anyway.

@compnerd

compnerd commented Mar 18, 2025

Copy link
Copy Markdown
Member

the release mode picked up more issues than the debug builds

Interesting. What kind of issues were these? Race conditions that weren’t hit in debug builds?

These actually weren't race conditions. There were a set of failures that involved memory corruption and runtime state corruption that only triggered in release mode sequential execution. These were amongst the more annoying ones to fix when the swift-corelibs-foundation test suite was initially enabled on Windows.

The other thing I noticed was running tests sequentially uncovered even more issues than running them in parallel

Since running the tests only takes ~1 minute, I don’t think we gain much by running them in parallel anyway. Incidentally, I found that running swift-format tests (also ~1 minute) is slightly faster in serial mode than in parallel mode. Probably because there’s less process launching overhead involved. So, I would keep tests in serial mode anyway.

Sounds good!

A 35 minute build time reduction is pretty good. I think that we should coordinate this with Mishal to see if we can do release mode tests in nightlies and debug mode tests in PR testing.

Edit: because I am greedy - is there any chance of getting any more time reductions by sharing build artifacts to avoid a full build like we do with Sourcekit-LSP, swift-format, etc?

…on Windows

This should improve CI times because building in debug configuration instead of release configuration is significantly faster. Since we don’t install the build of swift-(corelibs-)foundation using SwiftPM into the toolchain, this doesn’t have any performance impact on users of the toolchain.
@ahoppen
ahoppen force-pushed the foundation-tests-debug branch from 3e49537 to 3557451 Compare March 18, 2025 21:11
@ahoppen

ahoppen commented Mar 18, 2025

Copy link
Copy Markdown
Member Author

@swift-ci Please smoke test Windows

@ahoppen

ahoppen commented Mar 18, 2025

Copy link
Copy Markdown
Member Author

These actually weren't race conditions. There were a set of failures that involved memory corruption and runtime state corruption that only triggered in release mode sequential execution. These were amongst the more annoying ones to fix when the swift-corelibs-foundation test suite was initially enabled on Windows.

A 35 minute build time reduction is pretty good. I think that we should coordinate this with Mishal to see if we can do release mode tests in nightlies and debug mode tests in PR testing.

I added an option to build.ps1 to toggle the configuration. I will coordinate with Mishal to see if we can use that to test in release mode during the nightly runs.

Edit: because I am greedy - is there any chance of getting any more time reductions by sharing build artifacts to avoid a full build like we do with Sourcekit-LSP, swift-format, etc?

I want to share the build artifacts as well (if only to make sure that we actually test the libraries that get installed into the toolchain) but @jmschonfeld said that there have been issues in the past where a SwiftPM change made it impossible to build swift-foundation on Windows, so they want to continue building swift-foundation using SwiftPM in Windows CI to ensure that doesn’t break, essentially testing SwiftPM, not swift-foundation. Thus, I think we need to continue building swift-foundation on Windows using SwiftPM – but we might be able to use --multiroot-data-file to make sure we don’t build swift-syntax and swift-foundation twice (once to test swift-foundation and once for swift-corelibs-foundation) to save an additional 5 minutes.

Also, re-using the CMake build artifacts means that they need to stop using @testable imports. I started that migration here: swiftlang/swift-foundation#1215.

@ahoppen

ahoppen commented Mar 19, 2025

Copy link
Copy Markdown
Member Author

@swift-ci Please smoke test macOS

@ahoppen

ahoppen commented Mar 19, 2025

Copy link
Copy Markdown
Member Author

@swift-ci Please smoke test Linux

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.

3 participants