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

Set the default optimization level to 3 for testing #1443

Closed
wants to merge 1 commit into from

Conversation

daira
Copy link
Contributor

@daira daira commented Jul 7, 2024

Use --profile=dev to disable this. (As well as the fact that much of our code essentially requires optimization to run at all reasonably, this ensures that we are testing the code as it is compiled for release builds.)

An advantage of this over using --release when running tests is that it builds with full debug information (as far as possible given that some things will be optimized out).

to disable this. (As well as the fact that much of our code essentially
requires optimization to run at all reasonably, this ensures that we are
testing the code as it is compiled for release builds.)

Signed-off-by: Daira-Emma Hopwood <[email protected]>
@daira daira added the testing label Jul 7, 2024
Copy link

codecov bot commented Jul 7, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 63.22%. Comparing base (34cf6d2) to head (1b9ff59).
Report is 428 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1443   +/-   ##
=======================================
  Coverage   63.22%   63.22%           
=======================================
  Files         128      128           
  Lines       14865    14865           
=======================================
  Hits         9398     9398           
  Misses       5467     5467           

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

@@ -123,6 +123,10 @@ lto = true
panic = 'abort'
codegen-units = 1

[profile.test]
opt-level = 3
Copy link
Contributor

Choose a reason for hiding this comment

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

We should experiment with Cargo profile overrides to just optimize the math-heavy dependencies: https://docs.rust-embedded.org/book/unsorted/speed-vs-size.html#optimizing-dependencies

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not necessary to block making this change (which is also in #1603) on doing that. We can do so as well to further improve build+test times, but this is already a strict improvement on both the time and debugging QoL axes over --release, and you can override it with --profile=dev when you need to.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Filed as #1606.

@daira
Copy link
Contributor Author

daira commented Nov 2, 2024

Superceded by #1603.

@daira daira closed this Nov 2, 2024
@daira daira deleted the run-tests-with-optimization branch November 2, 2024 19:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants