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

[cmake] Allow enabling LTO for Qt frontend and libcelestia.so #1955

Merged
merged 4 commits into from
Nov 6, 2023
Merged

Conversation

375gnu
Copy link
Collaborator

@375gnu 375gnu commented Oct 27, 2023

Main code and Qt5 frontend benefit from LTO. Gtk+ and SDL frontends increase their sizes, i think it's due to statically linked libfmt.

Component Without LTO With LTO
libcelestia 4367840 3777176
celestia-qt5 1207152 1112024
celestia-gtk 656136 772136
celestia-sdl 439704 588576

g++ is 7.5.0. Other compilers may give other numbers.

With g++12.2 and 1.10.1:

Component Without LTO With LTO
libcelestia 4253848 4547936
celestia-qt6 875240 825368

@375gnu
Copy link
Collaborator Author

375gnu commented Oct 27, 2023

MSVC doesn't like something.

@ajtribick
Copy link
Collaborator

Tried this locally, it works with the Ninja generator but not the Visual Studio one. Need to figure out why but supplying -G "Ninja" in the cmake parameters in ci.yml might fix it.

@ajtribick
Copy link
Collaborator

Ok further investigation - Ninja doesn't work either, but it was passing a build configuration of RelWithDebInfo while MSVC was translating this to Release, so Ninja didn't use LTO.

The reason appears to be that WINDOWS_EXPORT_ALL_SYMBOLS is not compatible with LTO for DLLs - we'd probably have to mark the relevant functions with the appropriate __dllspec attributes, or compile the celestia library as static rather than shared.

In member function ‘addTreeItemChildrenFiltered’,
    inlined from ‘createTreeItem’ at src/celestia/qt/qtsolarsystembrowser.cpp:280:36:
src/celestia/qt/qtsolarsystembrowser.cpp:349:51: warning: argument 1 value ‘18446744073709551615’ exceeds maximum object size 9223372036854775807 [-Walloc-size-larger-than=]
  349 |     item->children = new TreeItem*[item->nChildren];
Copy link

sonarqubecloud bot commented Nov 5, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@375gnu 375gnu changed the title [cmake] Enable LTO for release builds [cmake] Allow enabling LTO for Qt frontentend and libcelestia.so Nov 6, 2023
@375gnu 375gnu changed the title [cmake] Allow enabling LTO for Qt frontentend and libcelestia.so [cmake] Allow enabling LTO for Qt frontend and libcelestia.so Nov 6, 2023
@375gnu 375gnu merged commit 0b8ef44 into master Nov 6, 2023
11 checks passed
@375gnu 375gnu deleted the lto branch November 6, 2023 18:40
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