perf(uv): load wheel metadata lazily#1348
Conversation
|
|
tamird
left a comment
tamird
left a comment
There was a problem hiding this comment.
The lazy-metadata split matches the OpenAI patch, but the current CI failures are causal and the universal-wheel path now pays the extra repository/target cost without a demonstrated win. Please refresh the affected snapshots/legacy target assumptions and cover the selector behavior before landing.
| if install_cfg.post_install_patches: | ||
| variant_kwargs["post_install_patches"] = json.encode(install_cfg.post_install_patches) | ||
| variant_kwargs["post_install_patch_strip"] = install_cfg.post_install_patch_strip | ||
| exclude_glob = getattr(install_cfg, "exclude_glob", []) |
There was a problem hiding this comment.
This creates a selector repository even when the lock has one universal wheel. The accompanying benchmark grows from 451 to 556 loaded packages and 8,677 to 9,097 configured targets without a measured speedup. Can we keep the single-variant path direct and split only genuinely platform-varying installs? Please also port the shared-alias, both-platform, and sdist-fallback regressions from the OpenAI patch.
| patches = repr(post_install_patches), | ||
| strip = post_install_patch_strip, | ||
| ) | ||
| exclude_glob = getattr(repository_ctx.attr, "exclude_glob", []) |
d0b8092 to
8d628b0
Compare
|
Alternative: #1356 |
8d628b0 to
3268a30
Compare
|
The Bazel 8/9 failures are causal: the extra variant repositories change the observable install roots and invalidate the venv/image snapshots and legacy wheel-chain fixture, while increasing the configured graph. #1356 keeps the stable install tree and avoids that growth, so it is the better metadata boundary. I reviewed its remaining provider/downloader gaps in #1356 (review); once those are addressed, this approach should be retired rather than updating snapshots around the path change. |
4 participants
Summary
Validation