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

Use the cargo-semver-checks GitHub Action for semver-checking #5648

Merged
merged 1 commit into from Apr 24, 2023

Conversation

obi1kenobi
Copy link
Contributor

Motivation

Will provide a substantial performance improvement:

  • cargo-semver-checks v0.20 is orders of magnitude faster at checking large crates — 2354x faster on aws-sdk-ec2
  • the GitHub Action caches baseline rustdoc JSON files which is worth another ~2x speedup.

As discussed here: #5437 (comment)

Solution

Switch to using the cargo-semver-checks GitHub Action which handles all of the above correctly and automatically. It also automatically downloads a prebuilt binary for the latest release of cargo-semver-checks so it should essentially run on autopilot from here on.

Will provide a substantial performance improvement:
- cargo-semver-checks v0.20 is orders of magnitude faster at checking large crates — 2354x faster on `aws-sdk-ec2`;
- the GitHub Action caches baseline rustdoc JSON files which is worth another ~2x speedup.

As discussed here: tokio-rs#5437 (comment)
@Darksonn Darksonn added the A-ci Area: The continuous integration setup label Apr 24, 2023
@Darksonn
Copy link
Contributor

The circle ci failure is unrelated. It was a network failure when downloading the assets needed to run the check. I have restarted it.

@obi1kenobi
Copy link
Contributor Author

obi1kenobi commented Apr 24, 2023

Before cargo-semver-checks v0.19: link

    Checking tokio v1.27.0 -> v1.27.0 (no change)
   Completed [  25.784s] 43 checks; 43 passed, 0 unnecessary

With cargo-semver-checks v0.20: link

    Checking tokio v1.27.0 -> v1.27.0 (assume minor change)
   Completed [   0.254s] 37 checks; 37 passed, 6 unnecessary

This is a bit over 100x faster. The underlying algorithm changed from O(n^2) to O(n) so larger crates get a bigger speedup.

Since this is the first run, this was also a cache miss — otherwise the log would have said cache hit next to the baseline rustdoc generation step. Subsequent runs will reuse the generated baseline rustdoc JSON files and will be even faster.

That reported number is just checking time, not rustdoc generation or parsing time. We have an open issue to improve how we report timing information: obi1kenobi/cargo-semver-checks#298

@taiki-e
Copy link
Member

taiki-e commented Apr 24, 2023

It appears to be the almost same performance as the most recent run on master.
https://github.com/tokio-rs/tokio/actions/runs/4787097411/jobs/8511915703

    Checking tokio v1.27.0 -> v1.27.0 (assume minor change)
   Completed [   0.214s] 37 checks; 37 passed, 6 unnecessary

@obi1kenobi
Copy link
Contributor Author

Sorry, my labeling was confusing — updated now. The current master also downloaded and used the v0.20 cargo-semver-checks release, so it taking about the same time is expected. But master doesn't cache baseline rustdoc JSON files, so a re-run of this PR's semver stage should result in a cache hit and make it faster still (even though that speedup won't change the number in the logs, because that's just rule-evaluation time and not total time: obi1kenobi/cargo-semver-checks#298).

@taiki-e
Copy link
Member

taiki-e commented Apr 24, 2023

Ah, thanks for the explanation. This looks good to me.

@Darksonn Darksonn merged commit a86c052 into tokio-rs:master Apr 24, 2023
53 checks passed
@obi1kenobi obi1kenobi deleted the patch-1 branch April 25, 2023 04:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ci Area: The continuous integration setup
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants