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

Add .cargo/config.toml with rustflags = -O to optimize test runtimes #3045

Closed
wants to merge 3 commits into from

Conversation

dconnolly
Copy link
Contributor

Motivation

When writing tests for #2645 , I created some tests that took over 5 minutes to run. When run with the rustc -O flag, they take ~30 seconds to run.

Solution

Add a .cargo/config.toml config file to provide -O as the rust flags for all compilations for our builds by default.

Review

@jvff @teor2345

@dconnolly dconnolly added A-rust Area: Updates to Rust code C-enhancement Category: This is an improvement P-Medium labels Nov 9, 2021
@dconnolly dconnolly added this to the 2021 Sprint 22 milestone Nov 9, 2021
@dconnolly dconnolly requested review from jvff and teor2345 November 9, 2021 23:44
@dconnolly dconnolly self-assigned this Nov 9, 2021
@teor2345
Copy link
Contributor

Some of the jobs are slower, but they should be faster once our build cache has cached an optimised build.

@teor2345
Copy link
Contributor

It looks like we're going over 75 minutes for Windows. That's because we're optimising a whole bunch of code we don't use much in the tests.

I'll try with optimisation level 1 now (-O means optimisation level 2, which is slower to build):
https://doc.rust-lang.org/rustc/codegen-options/index.html#opt-level

We can also restrict the optimisation to just the halo2 crate. But I'm not sure how we automatically optimise its dependencies as well.

https://doc.rust-lang.org/cargo/reference/config.html#profilenamepackagename

@teor2345
Copy link
Contributor

The stateful tests will override these settings, because environmental variables take precedence over configs:
https://doc.rust-lang.org/cargo/reference/config.html#environment-variables

So they'll continue to build with the higher optimisation level.

Copy link
Contributor

@teor2345 teor2345 left a comment

Choose a reason for hiding this comment

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

I don't think turning on optimisations is going to work. We've tried optimisation levels 1 & 2, and they both make the builds much longer.

Let's serialize the generated proofs instead.

We can use the Halo2Proof consensus serialization format, which is implemented here:

impl ZcashSerialize for Halo2Proof {
fn zcash_serialize<W: io::Write>(&self, writer: W) -> Result<(), io::Error> {
zcash_serialize_bytes(&self.0, writer)
}
}

Here's how we load data from a hex-encoded file in a lazy_static! block:

pub static ref BLOCK_MAINNET_GENESIS_BYTES: Vec<u8> =
<Vec<u8>>::from_hex(include_str!("block-main-0-000-000.txt").trim())
.expect("Block bytes are in valid hex representation");

Then we can deserialize typed data using code like:

let genesis_block: Arc<Block> = zebra_test::vectors::BLOCK_MAINNET_GENESIS_BYTES
.zcash_deserialize_into()
.unwrap();

The serialization should be the reverse of these steps, let me know if you have any trouble!

@dconnolly dconnolly closed this Nov 10, 2021
@dconnolly dconnolly deleted the cargo-config branch November 10, 2021 16:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rust Area: Updates to Rust code C-enhancement Category: This is an improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants