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

Removing strip = true from release profile. #5166

Merged
merged 3 commits into from
Nov 6, 2024
Merged

Conversation

dessalines
Copy link
Member

@dessalines dessalines commented Nov 5, 2024

Adding another profile optimization.

See https://github.com/johnthagen/min-sized-rust?tab=readme-ov-file#abort-on-panic for context.

@Nutomic
Copy link
Member

Nutomic commented Nov 5, 2024

This can make it much harder to debug crashes in production. Though in fact thats already tricky because we set strip = 0 which removes debug info for backtraces. So I would go the other way and disable stripping in release builds.

https://doc.rust-lang.org/cargo/reference/profiles.html#strip

@phiresky
Copy link
Collaborator

phiresky commented Nov 5, 2024

What's the reason that we are trying to optimize binary size so much? I'd generally agree that having more info for debugging release builds is good

@dessalines
Copy link
Member Author

Yep I did this because the strip removed the stack trace anyway, but if ppl prefer having them both, I'll go the other way with this.

@dessalines dessalines changed the title Adding abort on panic to release profile to make binary smaller. Removing strip = true from release profile. Nov 5, 2024
@Nutomic
Copy link
Member

Nutomic commented Nov 6, 2024

Confirmed that it works to produce backtraces (with RUST_BACKTRACE=1).


Error: LemmyError { message: Unknown(""), inner: Unknown

Stack backtrace:
0: anyhow::error::<impl anyhow::Error>::msg
1: <lemmy_utils::error::LemmyError as core::convert::From<lemmy_utils::error::LemmyErrorType>>::from
2: lemmy_server::create_http_server
3: lemmy_server::start_lemmy_server::{{closure}}
4: tokio::runtime::park::CachedParkThread::block_on
5: tokio::runtime::runtime::Runtime::block_on
6: lemmy_server::main
7: std::sys::backtrace::__rust_begin_short_backtrace
8: std::rt::lang_start::{{closure}}
9: std::rt::lang_start_internal
10: main
11: <unknown>
12: __libc_start_main
13: _start, context: Backtrace [{ fn: "<lemmy_utils::error::LemmyError as core::convert::From<lemmy_utils::error::LemmyErrorType>>::from" }, { fn: "lemmy_server::create_http_server" }, { fn: "lemmy_server::start_lemmy_server::{{closure}}" }, { fn: "tokio::runtime::park::CachedParkThread::block_on" }, { fn: "tokio::runtime::runtime::Runtime::block_on" }, { fn: "lemmy_server::main" }, { fn: "std::sys::backtrace::__rust_begin_short_backtrace" }, { fn: "std::rt::lang_start::{{closure}}" }, { fn: "std::rt::lang_start_internal" }, { fn: "main" }, { fn: "__libc_start_main" }, { fn: "_start" }] }

@Nutomic Nutomic merged commit d162ec1 into main Nov 6, 2024
2 checks passed
@SleeplessOne1917 SleeplessOne1917 deleted the profile_abort_on_panic branch November 6, 2024 13:47
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.

3 participants