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

Use as default thin lto instead to use full lto #748

Closed
wants to merge 5 commits into from

Conversation

NextWork123
Copy link

Hello, I noticed issue #737, and after some investigation, I would like to suggest switching from full LTO to ThinLTO. This change will optimize the compilation process more effectively than full LTO, resolve the compilation issues associated with it, and speed up the overall compilation time.

Best Regards,
NextWorks

* It gonna optimize more than full and it gonna resolve all the issue of compiling with lto and it gonna speedup the compiling process

* That commit point to fix the issue of #737
@orhun
Copy link
Contributor

orhun commented Nov 2, 2024

Any idea why thin LTO resolves the compilation issues on Arch Linux?

FWIW: briansmith/ring#1444

@NextWork123
Copy link
Author

NextWork123 commented Nov 2, 2024

Any idea why thin LTO resolves the compilation issues on Arch Linux?

FWIW: briansmith/ring#1444

some dependencies isn't passing with full lto (because of undefined symbols) so we need to switch to thin seem much packages on aur use thin as patch aswell i can put a patch in the pkgbuild and we can leave the full lto on all project errors about full lto:

https://paste.cachyos.org/p/eef0fd6.txt

let me know what i need to do

edit: i'm trying anyway to modify the pkgbuild of the guy for do the package rio-git, anyway i don't think is worth to use the flag lto it append the -flto so it enable the full lto and isn't worth too for the project the full lto we can use generally the thin is same as perfomance and less time for compile

what i think about your issue isn't needed to use options=(lto) if on cargo.toml is already true because it gonna enable the fat lto of rust

false: Performs “thin local LTO” which performs “thin” LTO on the local crate only across its codegen units. No LTO is performed if codegen units is 1 or opt-level is 0.
true or "fat": Performs “fat” LTO which attempts to perform optimizations across all crates within the dependency graph.
"thin": Performs “thin” LTO. This is similar to “fat”, but takes substantially less time to run while still achieving performance gains similar to “fat”.
"off": Disables LTO.

https://github.com/briansmith/ring/blob/main/Cargo.toml#L206 (they put lto=true) so if i'm correct on the pkgbuild we need to put options=(!lto) and leave the project with the fat lto

i tried today to recompile the pkgbuild with the option=(!lto) and leave the fat lto of rust do all the rest and seem working fine seem the deps fixed all undefined symbol but idk if is worth to use the fat lto or switch to thin.

We need to consider the fat lto gonna try to do lto on deps too if the deps have issues we can't compile it like yesterday :)

@orhun
Copy link
Contributor

orhun commented Nov 2, 2024

Yup, I think this should be solved on the Arch side of things instead of upstream.

We can keep the fat LTO and use the workaround mentioned in briansmith/ring#1444 - that's what we do for most of the Arch packages these days.

@NextWork123
Copy link
Author

NextWork123 commented Nov 2, 2024

Yup, I think this should be solved on the Arch side of things instead of upstream.

We can keep the fat LTO and use the workaround mentioned in briansmith/ring#1444 - that's what we do for most of the Arch packages these days.

yeah, we can do that or switch the project to thin lto and remove the fat lto (isn't much safe as i said it do too lto on deps and other)

or do a little patch on the pkgbuild to remove lto=true and replace with thin and you gonna resolve too

@NextWork123
Copy link
Author

I gonna close the pull request as resolve for atleast arch linux management we can leave fat lto as default and leave the package managers fix that.

@NextWork123 NextWork123 closed this Nov 2, 2024
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