We really need to stop with these out-of-tree proto files :/. They're constantly a cause of CI and build times issues because we have to "force" rebuild them sometimes and its non-trivial to monitor because they're not co-located with the build.rs that's (ab)using them.

Looking at the CI build/tests it also looks like the rust-cache isn't working.

Though that might be because it includes the profile in the cache key, and that was changed.

You might want to re-run the actions to see if that's the case. Although I think I set the cache to only save on push into next to minimize the cache size.

Given that it is also not necessary to specifically compile winterfell with a higher opt-level because it doesn't bring any additional benefits.

Is debug_assertions enabled at this level? The reason I'm asking is that winterfell runs a pretty expensive check (i.e., here and here) when debug_assertions are enabled.

winterfell runs a pretty expensive check [...] when debug_assertions are enabled.

Ah, thank you. We were enabling debug-assertions even before under the test-release profile because we explicitly enabled debug-assertions in the profile and also because we passed RUSTFLAGS="-C debug-assertions" on the command line.
Disabling the debug assertions for winter-prover specifically gives a massive boost in test execution time. On my machine make test-prove now compiles in 63s and runs in 11s.

The build time in CI is now similar to before with 6m 16s but test execution time is now 7m 58s, so a nice improvement over the original ~30 min.

I removed all DEBUG_ASSERTIONS from the Makefile because they are either implicitly enabled through the use of the standard cargo dev profile or through the cargo test-dev profile. I think it's clearer that way too. Previously I missed that we were overwriting this option on the cli. Now everything is specified in Cargo.toml.

I also combined make test-default and make test-prove because the prove tests are not significantly slower than the other ones, so the distinction no longer seems necessary. Let me know if we should still keep that distinction. (We still have make test-dev to run a subset of the most basic tests which I use locally as a sanity check.)

I tried using the mold linker in CI but there does not seem to be a significant difference, so I reverted that change to keep the setup as simple as it is.

By far the slowest test is now create_accounts_with_non_zero_anchor_block which doesn't do any proving. It is slow because it creates $2^{16}$ blocks with the MockChain to test an account ID creation with something other than the genesis block as the anchor. If we could speed up the creation of fake blocks in the mock chain that would help here. I guess the slow part comes from all the hashing in the MMR operations, not sure if we can cheat here somehow.

So, on your machine, the combined build + test time went from ~375 seconds to ~75 second, right? Or about 5x improvement?

Exactly, yes.

And on CI we went from ~30 mins to ~14 mins, right? So, about 2x improvement?

Ah no, when I said "test execution time is now 7m 58s" I should have said the "entire test time", not just the execution time. So it went from ~30mins to ~8mins, so 3.75x improvement.

So, on your machine, the combined build + test time went from ~375 seconds to ~75 second, right? Or about 5x improvement?

Exactly, yes.

And on CI we went from ~30 mins to ~14 mins, right? So, about 2x improvement?

Ah no, when I said "test execution time is now 7m 58s" I should have said the "entire test time", not just the execution time. So it went from ~30mins to ~8mins, so 3.75x improvement.

This should improve further once you land this on next due to caching.

This should improve further once you land this on next due to caching.

The tests on next now run in 3 minutes, compared to the previous 30 mins, which is pretty cool. And that is still slowed down by just one test (#1234).

The VM uses the approach we had previously, so maybe worth it wo change it there as well, cc @plafer.