-
-
Notifications
You must be signed in to change notification settings - Fork 70
Use new production=yes option #22
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
Conversation
Equivalent to debug_symbols=no use_lto=yes use_static_cpp=yes. We keep LTO disabled for iOS as users need to relink on deploy, and that's very slow and memory hungry with LTO.
# Keep LTO disabled for iOS - it works but it makes linking apps on deploy very slow, | ||
# which is seen as a regression in the current workflow. | ||
export OPTIONS="production=yes use_lto=no" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@naithar 3.2.4 RC 2 used this with OPTIONS="production=yes"
only.
I guess this change should fix godotengine/godot#46064.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about -flto=thin
, it should be supported and work much faster.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could try it, though that requires changes to platform/iphone/detect.py
. Might also be worth reworking how we pass LTO options as e.g. for -flto=thin
on Linux you need to pass use_lto=yes use_lld=yes use_thinlto=yes
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about
-flto=thin
, it should be supported and work much faster.
It was 3x faster for tvOS builds #21, so we can try using it. I can still provide macOS based non-lto builds just in case.
@akien-mga should I also change tvOS build script to support production
flag?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should I also change tvOS build script to support
production
flag?
Yeah, sounds good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about -flto=thin, it should be supported and work much faster.
And it's using much less memory (under 900 MB), waiting for a few minutes for release builds seems pretty reasonable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, LTO flags are completely missing from macOS detect.py
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I think we should overhaul the LTO flags to make sure they're consistent across all platforms and use the most optimal configuration by default (so thin
on Clang/LLD).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I opened godotengine/godot#46317 to continue the discussion on LTO for iOS and macOS.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had a small doubt whether this override would work... and it turns out it didn't :)
Needs some fixes to how we handle overriding options that would get default values from dev=yes
and production=yes
: godotengine/godot#46365
Equivalent to debug_symbols=no use_lto=yes use_static_cpp=yes.
We keep LTO disabled for iOS as users need to relink on deploy, and that's very slow
and memory hungry with LTO.