Skip to content

Speed up the pre-submission checks - #6153

Open
Manciukic wants to merge 4 commits into
firecracker-microvm:mainfrom
Manciukic:perf/verification-speedup
Open

Speed up the pre-submission checks#6153
Manciukic wants to merge 4 commits into
firecracker-microvm:mainfrom
Manciukic:perf/verification-speedup

Conversation

@Manciukic

@Manciukic Manciukic commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Changes

Three independent speedups to the pre-submission checks:

  • Pass pylint an explicit -j. Its jobs = 0 auto-detect reads cgroup v1
    cpu.shares, which Docker leaves at its 1024 default, so it lints on one
    core inside the dev container however many the host has.
  • Add the scientific stack to pylint's ignored-modules. pylint parallelises
    per file, so the slowest file bounds the whole run, and tools/ab_plot.py
    took 15.4s of a 15.8s run because astroid builds a full AST for matplotlib,
    numpy, pandas, scipy and seaborn.
  • Add checkstyle --no-clippy. checkstyle and checkbuild run the same
    clippy command and differ only in which triples they cover, so running both
    checks the native architecture twice.

Reason

checkstyle and checkbuild --all are what contributors run before pushing, so
their cost is paid on every commit. Measured on a 96-core host with a warm cargo
cache:

before, sequential before, concurrent after, sequential after, concurrent
no source change 41.7s 43.5s 18.9s 17.5s
one Rust file changed 49.0s 39.9s 35.2s 19.1s

CI's style step runs checkstyle on its own, which goes from 39.0s to 16.4s
with no source change. pylint was 31.7s of that 39.0s.

--no-clippy does not remove clippy work, it moves the native run to
checkbuild, which then no longer inherits a warm cache from checkstyle
(9.6s to 18.4s). The gain is in not doing that run twice.

It also stops the two commands clippying the same triple, which is what makes
running them concurrently worthwhile. Before, concurrency still helped when
something had changed, because checkstyle was long enough to hide
checkbuild underneath it, but it cost time when nothing had, since the two
contend for the native triple and cargo holds a lock on the build directory for
the whole build. After, the two share no work and the concurrent column is the
fastest in both rows.

License Acceptance

By submitting this pull request, I confirm that my contribution is made under
the terms of the Apache 2.0 license.

PR Checklist

  • I have read and understand CONTRIBUTING.md.
  • I have run tools/devtool checkbuild --all to verify that the PR passes
    build checks on all supported architectures.
  • I have run tools/devtool checkstyle --no-clippy to verify that the PR
    passes the automated style checks.
  • I have described what is done in these changes, why they are needed, and
    how they are solving the problem in a clear and encompassing way.
  • I have updated any relevant documentation (both in code and in the docs)
    in the PR.
  • I have mentioned all user-facing changes in CHANGELOG.md. (None: this
    only touches developer tooling.)
  • If a specific issue led to this PR, this PR closes the issue.
  • When making API changes, I have followed the
    Runbook for Firecracker API changes.
  • I have tested all new and changed functionalities in unit tests and/or
    integration tests. (No new tests: the changed code is the test tooling
    itself, and each commit was verified by running checkstyle on it alone.)
  • I have linked an issue to every new TODO.

  • This functionality cannot be added in rust-vmm.

@codecov

codecov Bot commented Aug 25, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.99%. Comparing base (761f88f) to head (3e1f236).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6153   +/-   ##
=======================================
  Coverage   82.99%   82.99%           
=======================================
  Files         277      277           
  Lines       30920    30920           
=======================================
  Hits        25663    25663           
  Misses       5257     5257           
Flag Coverage Δ
5.10-m5n.metal 83.27% <ø> (ø)
5.10-m6a.metal 82.63% <ø> (+<0.01%) ⬆️
5.10-m6g.metal 80.07% <ø> (ø)
5.10-m6i.metal 83.26% <ø> (-0.01%) ⬇️
5.10-m7a.metal-48xl 82.61% <ø> (-0.01%) ⬇️
5.10-m7g.metal 80.07% <ø> (ø)
5.10-m7i.metal-24xl 83.23% <ø> (-0.01%) ⬇️
5.10-m7i.metal-48xl 83.24% <ø> (-0.01%) ⬇️
5.10-m8g.metal-24xl 80.07% <ø> (ø)
5.10-m8g.metal-48xl 80.07% <ø> (-0.01%) ⬇️
5.10-m8i.metal-48xl 83.24% <ø> (ø)
5.10-m8i.metal-96xl 83.24% <ø> (ø)
6.18-m5n.metal 83.29% <ø> (-0.01%) ⬇️
6.18-m6a.metal 82.65% <ø> (-0.01%) ⬇️
6.18-m6g.metal 80.07% <ø> (ø)
6.18-m6i.metal 83.29% <ø> (ø)
6.18-m7a.metal-48xl 82.64% <ø> (+<0.01%) ⬆️
6.18-m7g.metal 80.07% <ø> (ø)
6.18-m7i.metal-24xl 83.30% <ø> (ø)
6.18-m7i.metal-48xl 83.30% <ø> (-0.01%) ⬇️
6.18-m8g.metal-24xl 80.07% <ø> (ø)
6.18-m8g.metal-48xl 80.07% <ø> (+<0.01%) ⬆️
6.18-m8i.metal-48xl 83.30% <ø> (-0.01%) ⬇️
6.18-m8i.metal-96xl 83.30% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

pylint's `jobs = 0` auto-detect reads cgroup v1 `cpu.shares`, which
Docker leaves at its 1024 default, so it lints on one core in the dev
container however many the host has. Takes the check from 32s to 16s.

Signed-off-by: Riccardo Mancini <mancio@amazon.com>
pylint parallelises per file, so the slowest file bounds the whole run.
tools/ab_plot.py took 15.4s of a 15.8s run, because astroid builds a
full AST for matplotlib, numpy, pandas, scipy and seaborn. Ignoring
them drops the check to 6.1s and reports the same messages: these
libraries resolve too dynamically for the member checks to fire.

Signed-off-by: Riccardo Mancini <mancio@amazon.com>
checkstyle and checkbuild run the same clippy command and differ only in
which triples they cover, so running both at commit time checks the
native architecture twice. Skip it in checkstyle and leave it to
checkbuild, which also covers the cross-compiled architecture.

The two then share no work, so running them concurrently takes a
Rust-touching verification from 37s to 19s. checkstyle keeps clippy by
default because CI runs no other clippy.

Signed-off-by: Riccardo Mancini <mancio@amazon.com>
@Manciukic
Manciukic force-pushed the perf/verification-speedup branch from d6ceeca to a6d8a0d Compare August 25, 2026 15:57
@Manciukic
Manciukic marked this pull request as ready for review August 25, 2026 16:37
@Manciukic Manciukic added the Status: Awaiting review Indicates that a pull request is ready to be reviewed label Aug 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Status: Awaiting review Indicates that a pull request is ready to be reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants