Skip to content

ci: new CI - #644

Merged
alejandro-vaz merged 17 commits into
servo:v2from
alejandro-vaz:new-ci
Sep 23, 2026
Merged

alejandro-vaz merged 17 commits into
servo:v2from
alejandro-vaz:new-ci

Conversation

@alejandro-vaz

@alejandro-vaz alejandro-vaz commented Sep 20, 2026

Copy link
Copy Markdown
Collaborator

this PR remakes CI and simplifies it a great deal

it is a replacement for the rust.yml workflow

it adds three workflows:

  • checks.yml => fast checks "everything is okay" for PRs. runs on PRs, merge queue, and manually
  • fuzzing.yml => fuzzing, runs on cron every dat at 00:00 UTC, and manually when requested

it deliberately changes tests/borsh.rs::round_trip_zst to have less elements so the miri test doesn't hang and reorders a block in src/taggedlen.rs so that there is no warning emitted

closes #654, it also removes dependabot.yml

checks

checks has seven jobs:

  • benchmarks: checks benchmarks compile, doesn't run them
  • practices: runs rustfmt, clippy, and documentation build
  • miri: runs all miri
  • stable: runs default testing
  • nightly: runs all testing
  • nostd: runs default testing
  • msrv: runs default testing

in total, these jobs are more comprehensive than our current checks

it removes the old beta testing. argument: why have it when we have both stable and nightly?? what's the point exactly?? it's the same as saying we should test every rust version since the MSRV or every nightly commit

it also removes the old "abort all other checks if one fails" so you can discover all issues at the same time

estimated run time: 1-2 minutes, around 8-10x faster than our current checks, which is perfect for PRs

fuzzing

fuzzing is currently being remade on #658

this PR adds the solicited cron job and separates it from regular checks

this probably should be updated once that PR lands

@alejandro-vaz alejandro-vaz self-assigned this Sep 20, 2026
@alejandro-vaz

Copy link
Copy Markdown
Collaborator Author

surprisingly, it is faster than the old one, because we removed 5-10 minutes of doing nothing in tests/borsh.rs

@alejandro-vaz alejandro-vaz changed the title ci: new checks file ci: new CI Sep 20, 2026
Signed-off-by: Alejandro Vaz <alejandro.vaz.myt@gmail.com>
@alejandro-vaz

Copy link
Copy Markdown
Collaborator Author

I also don't think it's the best idea to do either fuzzing or benchmarking on PRs. It takes a considerable amount of time (namely for the benchmarking part), the fuzzing is flaky/inconsistent, especially for a 30 secs run, and the benchmarking is probably noisy and unreliable given that it's running on GitHub Actions runners, where it probably shouldn't run. I feel like we should run an extensive, longer, proper fuzzing under a cron schedule and benchmark manually via comments or using dedicated runners instead. But this is an off-topic thing I noticed during the nightly action run and, if relevant, should probably have an issue of its own.

Originally posted by @pedrodesu in #658 (comment)

@alejandro-vaz

Copy link
Copy Markdown
Collaborator Author

we need some kind of baseline for benchmarks

if it is not in GitHub actions, then where??

that's my main argument

@alejandro-vaz
alejandro-vaz marked this pull request as ready for review September 22, 2026 16:56
@alejandro-vaz
alejandro-vaz requested a review from jdm September 22, 2026 16:56
@alejandro-vaz

alejandro-vaz commented Sep 22, 2026

Copy link
Copy Markdown
Collaborator Author

the motivation behind this: GitHub throttles CI minutes after 2k for the repo, because it's an org I think

we still have unlimited but sometimes GitHub refuses to give us enough parallel runners for no good reason

I think it's because we were consuming previously 30-40+ minutes of CI per run of rust.yml which have quickly added to 4k+ over the last month

for perspective, we have used ~6k minutes in the last year

@pedrodesu

Copy link
Copy Markdown
Contributor

we need some kind of baseline for benchmarks

if it is not in GitHub actions, then where??

that's my main argument

For local development of performance-critical work, the developer should bench in their own machine with cargo bench with --save-baseline and --baseline. Actions are inherently poor because of shared cloud VMs. They will be imprecise. This is also probably a good explanation on why we've noticed in few instances that benchmarks varied widely (assuming we were taking into account the action's results). The alternative would be to self-host a dedicated runner, but this costs money. Doesn't seem like a necessity for an open-source project where we can just measure ourselves in our own machines when messing with performance.

For PRs, do not run cargo bench. Run cargo bench --no-run merely to ensure that the benchmarks compile and live on through PRs, and run something like iai-callgrind instead. This is ideal for CI and it measures things such as instruction count and cache misses via cachegrind, not time. It is deterministic and doesn't care for noise. For something like GitHub actions that are shared VMs, this will be a far more accurate indicator of performance than Criterion. Leave that for local development, by the developer, on a dedicated machine.

the motivation behind this: GitHub throttles CI minutes after 2k for the repo, because it's an org I think

The motivation is very justifiable, I don't think we're using our 'action time' properly as of right now.

@alejandro-vaz
alejandro-vaz marked this pull request as draft September 22, 2026 17:32
@alejandro-vaz

Copy link
Copy Markdown
Collaborator Author

will see what can be done tomorrow

@alejandro-vaz
alejandro-vaz removed the request for review from jdm September 22, 2026 17:33
@alejandro-vaz

Copy link
Copy Markdown
Collaborator Author

I have no idea how to implement callgrind into CI, zero

I added the no-run compilations of benchmarks though

@alejandro-vaz

Copy link
Copy Markdown
Collaborator Author

I'll create another issue for that callgrind stuff honestly

@alejandro-vaz
alejandro-vaz marked this pull request as ready for review September 23, 2026 12:10
@alejandro-vaz
alejandro-vaz requested a review from jdm September 23, 2026 12:10
@pedrodesu

Copy link
Copy Markdown
Contributor

Alternatively, could be a good idea to check out codspeed instead of getting cachegrind/callgrind to work. Apparently some big OSS and non-OSS projects use it for the exact matter we're trying to improve here. Just some food for thought.

@alejandro-vaz
alejandro-vaz added this pull request to the merge queue Sep 23, 2026
Merged via the queue into servo:v2 with commit c5c2932 Sep 23, 2026
7 checks passed
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.

delete .github/dependabot.yml

3 participants