uv: remove --no-default-features from formula#222211
Conversation
github-actions
bot
added
autosquash
This disables the `performance` feature, which provides critical performance improvements for production use-cases
zanieb
force-pushed
the
zb/uv-default-features
branch
from
f97476b to
426781b
Compare
zanieb
changed the title
--no-default-features from uv formula--no-default-features from formula
github-actions
bot
removed
the
autosquash
carlocab
left a comment
carlocab
left a comment
There was a problem hiding this comment.
Using this flag disables the
performancefeature, which provides critical performance improvements for production use-cases.
Do you have benchmarks for these improvements?
|
I can generate some benchmarks, but we rigorously tested all of the performance optimizations included there when we added them in the first place. For quite some time (uv 0.1.0 -> 0.4.17), these were already enabled in Homebrew, until we moved them to a feature to allow us to toggle them off for development builds. |
|
Separately from the argument that this will improve performance (which I think may only apply to pathological cases we've optimized), we do not intend downstream consumers to build without the default features and it is quite feasible disabling the default features like this will lead to additional unexpected regressions in the future. |
#5577 fixed a bug on macos due to dynamically linking lzma/xz through static linking. In #7686, this feature was moved to the performance category. This PR moves the `xz2/static` back to the general default features, and, inspired by Homebrew/homebrew-core#222211, it structures and documents the feature flags cleaner
#5577 fixed a bug on macos due to dynamically linking lzma/xz through static linking. In #7686, this feature was moved to the performance category. This PR moves the `xz2/static` back to the general default features, and, inspired by Homebrew/homebrew-core#222211, it structures and documents the feature flags cleaner. We need to take care that this feature does not accidentally disable features we want. --------- Co-authored-by: Zanie Blue <contact@zanie.dev>
|
Our standard benchmarks actually don't show a major difference here, which is reassuring. However, I do believe there are pathological cases where the performance features are relevant. Additionally, we expect these features to be used in production and it could easily be confusing for us trying to help users. As an upstream maintainer, I strongly urge you to remove the |
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
|
👋 please don't stale bot this pull request |
|
cc @chenrui333 (you reviewed the original formula) |
branchv
changed the title
--no-default-features from formula--no-default-features from formula
|
thank you @zanieb, sorry for the delay. this will ship with the next |
|
Thank you! |
5 participants
Using this flag disables the
performancefeature, which provides critical performance improvements for production use-cases. I am unsure why it was ever included, it's been there since the formula was added.See https://github.com/astral-sh/uv/blob/481d05d8dfb8627612dec72840a02c17b926b263/crates/uv/Cargo.toml#L138-L146 for an enumeration of features.
See astral-sh/uv#7686 for the original motivation for splitting performance into a separate feature.