-
Notifications
You must be signed in to change notification settings - Fork 38
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
Improve documentation of partial solution #294
base: dev
Are you sure you want to change the base?
Conversation
Improve the docstrings for the partial solution and rename some fields to be easier to understand.
CodSpeed Performance ReportMerging #294 will degrade performances by 6.6%Comparing Summary
Benchmarks breakdown
|
That's a dubious looking perf regression |
/// The number of decisions that have been made, equal to the number of packages with decisions. | ||
current_decision_level: DecisionLevel, |
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.
"packages with decisions" being "packages with a decided version"?
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.
yes
src/internal/partial_solution.rs
Outdated
/// extract the solution, and to backtrack to a particular decision level. The | ||
/// `AssignmentsIntersection` is always a `Decision`. | ||
/// | ||
/// `[changed_this_decision_level..]`: Packages that are dependencies of some other package, |
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.
changed_this_decision_level
looks removed?
the only two structural changes are a tuple converted to a struct with field names, and another struct which has the order of its fields changed. Could it be byte alignement due to the new fields order? |
Co-authored-by: Zanie Blue <[email protected]>
Co-authored-by: Zanie Blue <[email protected]>
I can't reproduce this locally, and definitely not in the 6% range:
|
Improve the docstrings for the partial solution and rename some fields to be easier to understand.
Thanks to @Eh2406 for providing the context for these fields!