-
Notifications
You must be signed in to change notification settings - Fork 192
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update Windows image CI to Windows-2025 #511
Update Windows image CI to Windows-2025 #511
Conversation
@sbc100 pinging for a review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it important to test on two different version of windows? Presumably there is cost to each config we add/test.
I did it to match what I change the MacOS PR to #510 . If you mean in terms of monetary cost, no. For open source projects I've found you generally get unlimited minutes for Github runners for free. For the open source projects I maintain Github we use the runners labelled as generally available, and have never hit a limit imposed by Github. |
I think the limits can show up as throttling, and a general slowdown the CI throughput, but I'm not totally sure how github manage it. I also think maybe they do some kind of per-org metering too. We've been seeing WebAssembly/binaryen CI going super slow recently. I think as a general rule the more work you do in CI the slower you can expect your CI cycle to be (I think github have some kind of metering that makes this true even you do all of your work in parallel on many runners). |
You can get a slow down as an organisation if your hitting the limit of the maximum number of concurrent Github runners limit allowed (outlined here https://docs.github.com/en/actions/administering-github-actions/usage-limits-billing-and-administration ). These MacOS limit is generally very easy to hit, and you can be waiting a while for MacOS runner if many PRs in an organisation are simultaneously trying to use them. My suggestion is to change my Windows and MacOS PRs back to using the latest versions (which I have found to be slightly quicker). That means as a rule you'll get through each target system job quicker, using less minutes, and less runners overall. |
Personally I would agree with @sbc100 that we probably don't need both tested here (or for macos). It eats up comcurrency in the org for quite awhile without too too much benefit. I think it's reasonable to update the image though. |
Sometime later today I will revert my PRs back to using just updating to the latest image. |
8286755
to
abf69ad
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually I wonder if its better to keep testing with older version of windows to make sure we are compatible?
Do you have some reason to prefer/require this very latest version? I don't think wasi-libc should be depending it, right?
I wrote up some more thoughts here on that but IMO the risks aren't big enough either way to warrant adding more tests given CI costs at this time. |
f16cab5
to
2fbdc6b
Compare
@alexcrichton @sbc100 I've changed this PR and the MacOS PR to just change the image to the latest version, but not the name. That is the consensus I think has been reached. Let me know if it was decided to keep the older images for compatibility checks. |
Can you respond to #511 (review) ? Basically, I don't think we should be doing this unless there is good reason. |
@sbc100 If you look at the Windows job that just run on main the ci took over 2 hours to run on the Windows-2022 runner (see https://github.com/WebAssembly/wasi-sdk/actions/runs/13116480943/job/36591949707#step:12:1). If you look at the same job on the Windows-2025 runner here https://github.com/WebAssembly/wasi-sdk/actions/runs/13113946448/job/36594227539#step:12:1 it took around 40 minutes. That speed up seems worth it if your trying to save runner minutes, and effect the slowdown of other repos in the organisation. |
Thats interesting. Are you sure the differences is accounted for by the version of windows being used? It seems odd that 2025 would be so much faster. Do you know if that is expected? If so, please update the PR description explaining that 2025 is much faster (and if possible why its faster). |
I have no idea why its so much faster to be honest. I have had this in the past with other operating systems. This is normally due to an increase in the number of cpu cores available to the new runner, but cannot find any documentation on the number of cores of the separate runners, so on my part its complete speculation. I'll rebase the PR incase the speed difference is due to a recent change to main. |
2fbdc6b
to
a2a09ef
Compare
@sbc100 The times mentioned above turn out be some random fluke since I found this ci run where both ran side by side, and got the same time for both runners https://github.com/WebAssembly/wasi-sdk/actions/runs/13101673062/job/36550725190 . Will close both PRs if the updated images are believe not needed, and want to stick with older ones for compatibility reasons. I cannot give a specific reason for using the updated Windows version. I'm not a Windows developer, so couldn't tell you if there is any significant difference between the versions. |
Lets close this then. Better to continue to support reasonably old windows unless we have reason not to. |
Does the same apply to my MacOS PR? |
I think so yes, unless you have a specific reason that we shouldn't keep testing on older OS versions. |
Closing as it was chosen to keep older image in order to keep compatibility with older operating systems. |
This PR adds a Windows 2025 job to the ci, so that the 2 most recent versions of Windows server which are available as Github runners are supported by the ci.