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

perf: precompile contracts for faster test runs #14

Merged

Conversation

jaypaik
Copy link
Collaborator

@jaypaik jaypaik commented Nov 29, 2023

Motivation

IR compilation of the entire project takes a long time. When excluding the test suite however, the time cuts down dramatically. It would be nice to compile just the source contracts via IR and the test suite without. However, there's currently no way (in Foundry) to specify different profiles or different compilation configuration for specific files in a build.

Solution

The approach taken here is inspired by https://github.com/ProjectOpenSea/seaport. I've created a profile optimized-build for the purpose of precompiling just the source contracts via IR into the folder out-optimized. When tests are run under the optimized-test or optimized-test-deep profiles, the project is built again without IR, but the relevant source contracts are deployed using the optimized bytecode from out-optimized so the tests can test against the bytecode that is actually to be deployed.

I've updated the CI workflow to remove the lite test in favor of using the optimized test. It should be fast enough now that we don't need the early signal provided by the lite profile.

Before (3m 59s) After (58s)
Screenshot 2023-11-29 at 1 27 29 AM Screenshot 2023-11-29 at 1 27 10 AM

Please let me know if you see any flaws with this!

@jaypaik jaypaik force-pushed the 11-28-perf_precompile_contracts_for_faster_test_runs branch 2 times, most recently from dad76c9 to c4b0140 Compare November 29, 2023 06:25
@jaypaik jaypaik marked this pull request as ready for review November 29, 2023 06:30
Copy link
Contributor

@adam-alchemy adam-alchemy left a comment

Choose a reason for hiding this comment

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

Nice! This is awesome, and really clever solution.

One thing to add: I think we should keep the non-IR tests in the CI, even if we're not using the deep profile (though we could). In the past, we once had a test case that passed with non-IR but failed with IR, which is the opposite direction of what this would catch but just shows that it is possible for these differences to exist. I think in that case it was due to unsafe memory accesses in assembly, which creates undefined behavior.

Also had a small comment re: bytecode_hash

foundry.toml Outdated Show resolved Hide resolved
@jaypaik jaypaik force-pushed the 11-28-perf_precompile_contracts_for_faster_test_runs branch from c4b0140 to 4e9e56e Compare November 29, 2023 23:49
ignored_error_codes = [3628]
[profile.optimized-build]
via_ir = true
test = 'src'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this the line that skips building tests contract?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yup! The alternative is to use --skip test, but that ends up compiling files in the test directory that do not end in t.sol.

@jaypaik jaypaik force-pushed the 11-28-perf_precompile_contracts_for_faster_test_runs branch 3 times, most recently from 9eb3a65 to cc85a74 Compare November 29, 2023 23:58
@jaypaik jaypaik merged commit ca45d90 into spec-update-6 Nov 30, 2023
3 checks passed
@jaypaik jaypaik deleted the 11-28-perf_precompile_contracts_for_faster_test_runs branch November 30, 2023 00:17
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.

3 participants