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

Enable Link-Time Optimization (LTO) for the Rust part #1011

Merged
merged 1 commit into from
Jan 3, 2025

Conversation

zamazan4ik
Copy link
Contributor

Related to #1010 but doesn't completely resolve it

Copy link
Contributor

@arighi arighi left a comment

Choose a reason for hiding this comment

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

What is the unresolved part? The C schedulers? Also it'd be nice to have a longer description in the commit message (like a summary of what you wrote in #1010), thanks

@zamazan4ik
Copy link
Contributor Author

What is the unresolved part? The C schedulers?

Yep.

Also it'd be nice to have a longer description in the commit message (like a summary of what you wrote in #1010), thanks

Just did it :)

@@ -13,3 +13,6 @@ members = ["rust/scx_stats",
"scheds/rust/scx_layered",
"scheds/rust/scx_mitosis"]
resolver = "2"

[profile.release]
Copy link
Contributor

@hodgesds hodgesds Nov 29, 2024

Choose a reason for hiding this comment

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

Maybe it's better to add another profile instead:

[profile.release-lto]
inherits = "release"
lto = true

Copy link
Contributor

Choose a reason for hiding this comment

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

Personally I would prefer, that the default would use LTO, and for the CI we pass LTO off or so.

Copy link
Contributor

Choose a reason for hiding this comment

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

The meson flag can be added, then we activate this in PKGBUILD.

@ptr1337
Copy link
Contributor

ptr1337 commented Jan 1, 2025

What is pending here?
I think the default release profile should use LTO and the debug variant just disables it.

@zamazan4ik
Copy link
Contributor Author

zamazan4ik commented Jan 1, 2025

I was waiting for the final decision about how exactly LTO should be enabled for the project: in a dedicated profile or in the regular Release Cargo profile; do I need to patch Meson build scripts in this PR too, etc. As far as I understand, no final agreement has been reached yet.

@htejun
Copy link
Contributor

htejun commented Jan 2, 2025

I don't see a reason to not enable it by default on release builds. Let's enable it and see whether anyone complains.

@htejun htejun closed this Jan 2, 2025
@htejun htejun reopened this Jan 2, 2025
@htejun htejun enabled auto-merge January 2, 2025 22:14
Enable Link-Time Optimization (LTO) for Rust schedulers. It improves the
binary size and allows to a compiler perform more aggressive
optimizations.

More details in sched-ext#1010
auto-merge was automatically disabled January 3, 2025 12:15

Head branch was pushed to by a user without write access

@htejun htejun enabled auto-merge January 3, 2025 12:37
@htejun htejun added this pull request to the merge queue Jan 3, 2025
Merged via the queue into sched-ext:main with commit 423c860 Jan 3, 2025
23 checks passed
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.

6 participants