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

Disable debugging features #110

Closed
pentschev opened this issue Oct 17, 2022 · 6 comments · Fixed by #111
Closed

Disable debugging features #110

pentschev opened this issue Oct 17, 2022 · 6 comments · Fixed by #111
Labels
question Further information is requested

Comments

@pentschev
Copy link
Contributor

pentschev commented Oct 17, 2022

Comment:

I'm curious if there's a reason why the UCX package is not doing release builds, currently ucx/configure is used, but the UCX repo provides a ucx/contrib/configure-release file that disables some features that are generally better suited for a debug build, which may also be somewhat detrimental to performance.

Is there a reason to keep building in the way it is currently build, or should we consider doing release builds as described above? I'm happy to submit a patch for that, unless someone has a reason why we should keep the build as it currently is.

@pentschev pentschev added the question Further information is requested label Oct 17, 2022
@leofang
Copy link
Member

leofang commented Oct 17, 2022

I am sure @jakirkham would know better the reason, to me it's simply my lack of knowledge of the proper way to build UCX. I'd say PRs are definitely welcomed.

@pentschev
Copy link
Contributor Author

Thanks Leo for the quick reply, I opened #111 to address that.

@jakirkham
Copy link
Member

jakirkham commented Oct 18, 2022

Yeah changing this seems reasonable.

The main issue is to make sure UCX isn't changing ABI compatibility of the libraries on rebuild (this can happen as debug libraries sometimes use different symbols). If it does, we might need to think harder about this change to minimize breakage of existing binaries (or coordinate it with a new release).

Most configure scripts have an --enable-debug flag that gets passed to enable debug mode. One can pass --disable-debug mode to be explicit, but that would already be the default. Had assumed this was the case with UCX, but didn't realize something non-standard was done here (maybe there is a reason for this?). Asking if this can be improved on in issue ( openucx/ucx#8642 ).

@jakirkham
Copy link
Member

Digging deeper into configure-release, it appears it passes --disable-debug along with a few other flags related to logging and assertions back to configure.

@pentschev
Copy link
Contributor Author

I'm not sure whether ABI is changed or not to be honest. But I agree with you, maybe waiting for the next UCX release is more appropriate to reduce breakage. What would be the best way to ensure we don't forget about that when the new release is out?

@jakirkham
Copy link
Member

Maybe we should do this as part of 1.14.0?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants