[Windows] Use a multiroot data file to test (corelibs-)foundation on Windows - #80122
Conversation
|
@swift-ci Please test Windows |
|
@swift-ci Please smoke test |
3d7ce49 to
6053b15
Compare
|
@swift-ci Please smoke test |
|
@swift-ci Please smoke test macOS |
|
Odd, we are building less in the swift-corelibs-foundation test but it’s still slower. Attaching relevant portions of the build log and running again to see if there’s nondeterminism involved. |
|
@swift-ci Please test Windows |
1 similar comment
|
@swift-ci Please test Windows |
6053b15 to
3483edb
Compare
|
@swift-ci Please test Windows |
1 similar comment
|
@swift-ci Please test Windows |
3483edb to
a2e001a
Compare
|
@swift-ci Please test Windows |
1 similar comment
|
@swift-ci Please test Windows |
a2e001a to
c0a8227
Compare
|
@swift-ci Please test Windows |
|
Looking at the performance logs for this PR, I realized that #80082 accidentally changed swift-foundation to be tested in release configuration again. This restores it to be built in debug again. It also saves ~2 minutes in swift-corelibs-foundation through the multiroot data file. It’s less than I would have expected Before this PR:
With this PR (Log):
Running once more to see how stable these timings are. Edit: Timings are stable. |
|
@swift-ci Please test Windows |
bnbarham
left a comment
bnbarham
left a comment
There was a problem hiding this comment.
LGTM, but CC @compnerd and @jmschonfeld as well
jmschonfeld
left a comment
jmschonfeld
left a comment
There was a problem hiding this comment.
Should we do the same thing on Linux to keep our test environments consistent between the two platforms?
There was a problem hiding this comment.
Is $FoundationTestConfiguration defined somewhere?
There was a problem hiding this comment.
Yeah, defaults to debug:
.PARAMETER FoundationTestConfiguration
Whether to run swift-foundation and swift-corelibs-foundation tests in a debug or release configuration.
...
[ValidateSet("debug", "release")]
[string] $FoundationTestConfiguration = "debug",
Should we do the same thing on Linux to keep our test environments consistent between the two platforms?
I had thought we were, but... apparently not 🤔
There was a problem hiding this comment.
The use of Debug does scare me a bit - we release in release mode, not debug mode, and the optimizations can introduce issues (and was in fact how we learnt about CFString bridging failures). I would recommend that we test in release mode only. However, the shared build is a good idea.
There was a problem hiding this comment.
There was a problem hiding this comment.
Right, it would be okay to test in CI with debug and nightlies with Release (I actually would prefer that). I just don't see how we control that for the nightlies.
There was a problem hiding this comment.
The nightlies are currently passing through release, eg. https://ci-external.swift.org/job/swift-main-windows-toolchain/1517/ has:
... utils\build.ps1 ... -FoundationTestConfiguration release
Though it also has -Test swift,dispatch,xctest,lldb, so they're not actually being run...
c0a8227 to
eac6514
Compare
|
@swift-ci Please test Windows |
eac6514 to
5f45130
Compare
|
@swift-ci Please test Windows |
|
Measured again and this reduces the test time for swift-foundation + swift-corelibs-foundation by 15 minutes. |
5f45130 to
a4ad221
Compare
compnerd
left a comment
compnerd
left a comment
There was a problem hiding this comment.
I think that we should go ahead and merge this for now, but would love to have @shahmishal verify that we are testing in release mode in nightlies.
There was a problem hiding this comment.
Right, it would be okay to test in CI with debug and nightlies with Release (I actually would prefer that). I just don't see how we control that for the nightlies.
There was a problem hiding this comment.
Would be nice if this was commented - this is here due to a failure that has been notoriously difficult to replicate outside of CI and we cannot get details on what is going on there.
There was a problem hiding this comment.
Ah yeah, sorry I should have done that. @ahoppen would you mind moving the -j 1 to the end and adding something like:
# Running parallel causes a non-deterministic crash in CI only, see https://github.com/swiftlang/swift/issues/83606
?
…Windows We currently rebuild swift-syntax, swift-foundation-icu and swift-foundation twice: Once to test swift-foundation and once to test swift-corelibs-foundation. Using a unified build for both projects means that we only need to rebuild them once, saving ~5 minutes.
a4ad221 to
61f833c
Compare
|
@swift-ci Please smoke test |
|
@swift-ci Please test Windows |
…ionTestConfiguration` is set to `debug` This was discussed in swiftlang#80122 (comment) but got reverted by swiftlang#83693. Re-apply the fix.
4 participants
We currently rebuild swift-syntax, swift-foundation-icu and swift-foundation twice: Once to test swift-foundation and once to test swift-corelibs-foundation. Using a unified build for both projects means that we only need to rebuild them once, saving ~5 minutes.