Skip to content
This repository was archived by the owner on Oct 25, 2025. It is now read-only.

Conversation

@matthiasgoergens
Copy link
Contributor

@matthiasgoergens matthiasgoergens commented May 9, 2024

Shuffle our code around a bit to speed up the incremental build.

Our target here is to make time cargo test --no-run run faster after a small change to circuits.

At the moment on my machine this PR speeds that benchmark up from around 35s on main down to around 15s.

@matthiasgoergens matthiasgoergens changed the title Speed up incremental build: move clap-derive logic into its own crate Speed up incremental build May 9, 2024
@matthiasgoergens matthiasgoergens force-pushed the matthias/speed-up-cli-compiling branch from ef99615 to 0caf013 Compare May 9, 2024 04:35
@matthiasgoergens matthiasgoergens marked this pull request as ready for review May 9, 2024 04:35
@@ -0,0 +1,47 @@
pub mod nop;
Copy link
Contributor Author

@matthiasgoergens matthiasgoergens May 9, 2024

Choose a reason for hiding this comment

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

This file moved here from the CLI.

($test_name:ident, $file_name:tt) => {
#[test]
fn $test_name() -> Result<()> {
run_test(include_bytes!(concat!(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Need to check whether reading the files at runtime actually improves build speed.

criterion = { version = "0.5", default-features = false, features = ["html_reports", "plotters", "cargo_bench_support"] }

[patch.crates-io]
clio = { git = "https://github.com/0xmozak/clio.git" }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change can go into its own PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm all for improving compilation speed but seems like this comes at the cost of reordering our CLI logic in a somewhat confusing manner, I'm not sure if args should belong outside of the cli.

let elf = fs::read(concat!("riscv-testdata/testdata/", $file_name))
.expect("Should have been able to read the file");
run_test(&elf)
run_test(include_bytes!(concat!(
Copy link
Contributor

Choose a reason for hiding this comment

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

This probably adds compile time. rust-lang/rust#65818

matthiasgoergens added a commit that referenced this pull request May 9, 2024
Upgrade clio and remove a now-obsolete linter exception

Extracted from #1682
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants