Skip to content
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

Conversation

mcbarton
Copy link
Contributor

@mcbarton mcbarton commented Feb 2, 2025

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.

@mcbarton mcbarton changed the title Update Windows Github runner to Windows 2025 Add Windows 2025 job to ci Feb 2, 2025
@mcbarton
Copy link
Contributor Author

mcbarton commented Feb 2, 2025

@sbc100 pinging for a review.

Copy link
Member

@sbc100 sbc100 left a 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.

@mcbarton
Copy link
Contributor Author

mcbarton commented Feb 2, 2025

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.

@sbc100
Copy link
Member

sbc100 commented Feb 2, 2025

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).

@mcbarton
Copy link
Contributor Author

mcbarton commented Feb 2, 2025

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.

@alexcrichton
Copy link
Collaborator

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.

@mcbarton
Copy link
Contributor Author

mcbarton commented Feb 3, 2025

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.

@mcbarton mcbarton force-pushed the Update-Windows-Github-runner-to-Windows-2025 branch from 8286755 to abf69ad Compare February 3, 2025 12:54
@mcbarton mcbarton changed the title Add Windows 2025 job to ci Update Windows image CI to Windows-2025 Feb 3, 2025
Copy link
Member

@sbc100 sbc100 left a 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?

@alexcrichton
Copy link
Collaborator

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.

@mcbarton mcbarton force-pushed the Update-Windows-Github-runner-to-Windows-2025 branch from f16cab5 to 2fbdc6b Compare February 3, 2025 19:23
@mcbarton
Copy link
Contributor Author

mcbarton commented Feb 3, 2025

@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.

@sbc100
Copy link
Member

sbc100 commented Feb 3, 2025

Can you respond to #511 (review) ? Basically, I don't think we should be doing this unless there is good reason.

@mcbarton
Copy link
Contributor Author

mcbarton commented Feb 3, 2025

@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.

@sbc100
Copy link
Member

sbc100 commented Feb 3, 2025

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).

@mcbarton
Copy link
Contributor Author

mcbarton commented Feb 3, 2025

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.

@mcbarton mcbarton force-pushed the Update-Windows-Github-runner-to-Windows-2025 branch from 2fbdc6b to a2a09ef Compare February 3, 2025 19:52
@mcbarton
Copy link
Contributor Author

mcbarton commented Feb 3, 2025

@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.

@sbc100
Copy link
Member

sbc100 commented Feb 3, 2025

Lets close this then. Better to continue to support reasonably old windows unless we have reason not to.

@mcbarton
Copy link
Contributor Author

mcbarton commented Feb 3, 2025

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?

@sbc100
Copy link
Member

sbc100 commented Feb 3, 2025

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.

@mcbarton
Copy link
Contributor Author

mcbarton commented Feb 3, 2025

Closing as it was chosen to keep older image in order to keep compatibility with older operating systems.

@mcbarton mcbarton closed this Feb 3, 2025
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