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

Track priorities in a set and allow updating from uv #42

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

konstin
Copy link
Member

@konstin konstin commented Jan 24, 2025

In uv, we were updating package priorities without syncing them uv, so uv and pubgrub were getting out of sync. We couldn't previously do this because the way priority updates were done. Instead, we switch to tracking the packages whose derivations changed in a set, based on pubgrub-rs/pubgrub@dev...Eh2406:pubgrub:stop-prioritize. Since uv priorities don't depend on the ranges, we can speed this up further by not using this set in uv. In pubgrub, we can upstream this and use it as a correctness fix to also reprioritize when the conflict tracker changed, which is currently not handled.

Also exposes into_raw since we start using it on the uv side.

Copy link

codspeed-hq bot commented Jan 24, 2025

CodSpeed Performance Report

Congrats! CodSpeed is installed 🎉

🆕 6 new benchmarks were detected.

You will start to see performance impacts in the reports once the benchmarks are run from your default branch.

Detected benchmarks

  • backtracking_disjoint_versions (2.4 s)
  • backtracking_ranges (2 s)
  • backtracking_singletons (4.5 s)
  • large_case_u16_NumberVersion.ron (25.4 ms)
  • sudoku-easy (3.7 ms)
  • sudoku-hard (4.2 ms)

let current_decision_level = self.current_decision_level;
self.package_assignments
.get_range(self.prioritize_decision_level..)
.get_range(current_decision_level.0 as usize..)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems like we could directly iterate over prioritized_potential_packages and outdated_priorities instead of iterating over all of package_assignments. I think this method is only used by UV, so not sure what semantics you need.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we use this to make prefetches, and i think currently in a kind of inefficient way. we need to emit a prefetch at least the first time we see a package, and at most when its range changes; the previous and this was good enough in profiling for now.

@konstin konstin force-pushed the konsti/simplify-prioritization branch from f58a60f to 3fb04c3 Compare January 27, 2025 12:38
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.

2 participants