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

Improve the CI runtime #4056

Merged
merged 7 commits into from
Aug 6, 2024
Merged

Improve the CI runtime #4056

merged 7 commits into from
Aug 6, 2024

Conversation

Systemcluster
Copy link
Contributor

@Systemcluster Systemcluster commented Aug 6, 2024

Follow-up to #4051, see there for prior discussion.

This splits up the longest-running CI jobs into smaller jobs to improve the CI runtime.

The overall usage didn't go down much, it's still around 100 minutes, but now it's spread out over more jobs.
(It's gotten a bit more stable slightly under the 100 minute mark with this PR, but not much.)

Additional changes are commented below.

@daxpedda
Copy link
Collaborator

daxpedda commented Aug 6, 2024

I'm happy to give it a final review already as-is, up to you when its ready!

I believe the last three things we could do are:

  • Parallelize build_examples via bash somehow (or if you know your way around webpack maybe there is something there?).
  • Remove the test --all-features -p web-sys, it can be tested without all features, but we have to add the required features to dev-dependencies. Checking building with all features is already done in another part of the CI.
  • Cross-compile dist_windows from Linux, which should finally get us down to under 3 minutes (if the other two points work out!).

Could all be done in a follow-up btw.

Copy link
Contributor Author

@Systemcluster Systemcluster left a comment

Choose a reason for hiding this comment

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

I didn't get it under 6 minutes, but the later commits touch up some details that might still be worthwile.

I'm done with the CI for a while, feel free to modify and force push this branch if something should change. I'll leave any additional improvements for you or someone else (or for me at some later time 🙂).

@@ -364,6 +364,7 @@ pub async fn test_example(
};

conn.graceful_shutdown();
tokio::time::sleep(Duration::from_millis(100)).await;
Copy link
Contributor Author

@Systemcluster Systemcluster Aug 6, 2024

Choose a reason for hiding this comment

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

It looks like it didn't work, maybe it's not (just) here after all? I'm pretty sure it's always the last test, so maybe the waiting needs to happen at a higher level (before shutting down tokio runtime?).

.github/workflows/main.yml Outdated Show resolved Hide resolved
_package.json Show resolved Hide resolved
examples/add/webpack.config.js Show resolved Hide resolved
@Systemcluster Systemcluster marked this pull request as ready for review August 6, 2024 19:46
Copy link
Collaborator

@daxpedda daxpedda left a comment

Choose a reason for hiding this comment

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

"Run wasm-bindgen crate tests with various environment variables" could still be split up, that ones the longest now.

I'm happy to merge it as-is if we can split out the caching.

Copy link
Collaborator

@daxpedda daxpedda left a comment

Choose a reason for hiding this comment

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

Thanks for all the work!

@daxpedda daxpedda merged commit 2ea6ab1 into rustwasm:main Aug 6, 2024
34 checks passed
@Systemcluster Systemcluster deleted the ci-improve branch August 6, 2024 22:21
@Systemcluster
Copy link
Contributor Author

Parallelize build_examples via bash somehow (or if you know your way around webpack maybe there is something there?).

I know that there are many faster webpack alternatives (rollup, rspack or parcel as bundlers, or maybe bare swc or esbuild can do it as well) that might be easy to switch to, I haven't looked at the webpack setup in detail to be able to tell.

I assume just running the loop in parallel is a bit easier though. parallel or something like this would surely work.

@Systemcluster
Copy link
Contributor Author

I need to add (with this as a follow-up, and it only showing in linked CI runs) that adding sccache really doesn't seem to help much with the wasm-bindgen CI. The jobs it speeds up aren't a bottleneck, and it doesn't seem to speed up the slow jobs for various reasons. It also doesn't reduce the 100 minute usage much.
@daxpedda's comment in the closed PR is on point, other gains should be prioritized, unless I missed some configuration that would improve sccache here.

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.

2 participants