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

Need to fix compilation flag lost in CMake refactor #1005

Merged
merged 1 commit into from
Oct 22, 2024

Conversation

starseeker
Copy link
Contributor

Looks like we lost the O3 flag always being on. This was good for some fun trying to figure out why performance had dropped off so dramatically - turned out it was simply because prior builds had NEVER been without O3, so all builds - Debug or Release - were optimized.

On the other hand, there are arguments for and against O3 with debug builds. The optimizations complicate/obscure debugging, but it's also an order of magnitude slower running. Rather than be completely either or, this PR defines a MANIFOLD_OPTIMIZED variable that a user can specify to ON explicitly. Build types Release and RelWithDebInfo will always have O3, and for other cases it'll be up to whether the user has explicitly requested optimized building with MANIFOLD_OPTIMIZED.

Looks like we lost the O3 flag always being on.  This was good for some
fun trying to figure out why performance had dropped off so dramatically
- turned out it was simply because prior builds had NEVER been without
O3, so all builds - Debug or Release - were optimized.

On the other hand, there are arguments for and against O3 with debug
builds.  The optimizations complicate/obscure debugging, but it's also
an order of magnitude slower running.  Rather than be completely either
or, this PR defines a MANIFOLD_OPTIMIZED variable that a user can
specify to ON explicitly.  Build types Release and RelWithDebInfo will
always have O3, and for other cases it'll be up to whether the user has
explicitly requested optimized building with MANIFOLD_OPTIMIZED.
Copy link

codecov bot commented Oct 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.86%. Comparing base (d437097) to head (4e93c4e).
Report is 130 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1005      +/-   ##
==========================================
- Coverage   91.84%   85.86%   -5.99%     
==========================================
  Files          37       62      +25     
  Lines        4976     9604    +4628     
  Branches        0     1046    +1046     
==========================================
+ Hits         4570     8246    +3676     
- Misses        406     1358     +952     

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

Copy link
Owner

@elalish elalish left a comment

Choose a reason for hiding this comment

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

Thanks!

@elalish elalish merged commit 85675f2 into elalish:master Oct 22, 2024
21 checks passed
@starseeker starseeker deleted the cmake_flags_fix branch October 22, 2024 19:11
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.

2 participants