-
Notifications
You must be signed in to change notification settings - Fork 161
BUILD: Change default buildtype from debug to release #869
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
base: main
Are you sure you want to change the base?
BUILD: Change default buildtype from debug to release #869
Conversation
Signed-off-by: Michal Shalev <[email protected]>
👋 Hi michal-shalev! Thank you for contributing to ai-dynamo/nixl. Your PR reviewers will review your contribution then trigger the CI to test your changes. 🚀 |
@aranadive @karya0, what was the original reason for NIXL's default build type being debug? |
README.md
Outdated
|
||
```bash | ||
$ meson setup <name_of_build_dir> --buildtype=release | ||
$ meson setup <name_of_build_dir> |
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 would put also the explicit command here
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.
fixed
maybe this CI issue is related, because we build release now |
src/plugins/gpunetio/README.md
Outdated
|
||
By default NIXL is built with `buildtype=debug` option. This is ok for correctness and debugging. | ||
To run for performace (e.g. with NIXL bench) t's hightly recommended to build NIXL with `buildtype=release`. | ||
By default NIXL is built with `buildtype=release` option for optimal performance. |
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.
maybe keep the explicit recommendation for performance
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.
fixed
probably need to explicitly set debug for CI builds now. |
There is the following code in
I'm not sure that this is the correct approach. I think it would be better to have separate options to build tests and examples. It makes sense to build examples in release mode. |
CI should cover at least release mode, otherwise we will miss issues. Maybe there would be side effects for debugging, but we do not support core dump collection right now anyway. So I think we should add tests and examples compilation in release builds. But with care not to package them into wheels/crates. |
A drawback is that asserts would be compiled out in release mode, so they will not trigger in CI if we build in release mode. |
Please see related #872 |
Signed-off-by: Michal Shalev <[email protected]>
/build |
Signed-off-by: Michal Shalev <[email protected]>
What?
Changes the default build type from debug to release.
Why?
Users should get optimized, production-ready builds by default without needing to specify build flags. Debug builds remain available with
--buildtype=debug
.How?
Updated
meson.build
default_options and documentation (README.md, gpunetio plugin README) to reflect release as the new default.