Backport CUDF trimming to the 2.0 branch #4538
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This appears to give huge solver speedups with MCCS on some tests
It's not what gives the most speedup, and it could be harmful for maximisation / best-effort requests.
Determines how far we go through the dependencies of the specified packages to extract conflicts. Defaults to 2, 0 disables conflict detection, more than 5 is probably meaningless. Higher values make the preprocessing take around 2s, but that could probably be optimised.
Computation of dependencies was based on `OpamCudf.Graph.close_and_linearize`, which is using a topological traversal of the dependency graph. Unfortunately, when cycles appear (which is now allowed with `post` dependencies, but could also happen before with malformed opam repos), this can cause dependencies to be missed. I believe this could manifest mysteriously in several bugs, but I dug it up on an issue about `opam remove -a` no longer working, reporting a conflict while attempting to remove `ocaml-config`. Note 1: there are small changes in the API for simplicity's sake: the dependency functions now return sets, and there is a separate sort function. We didn't use the order anyway and always converted to sets, except in one `opam list` case where we actually needed only the sort and not the dependency computing. Note 2: I used a conservative approach (fix the bug first), but it would make much sense to compute the cone directly from `OpamSolver` without going through `OpamCudf` and back, which is quite coslty.
- restriction to the cone of mentionned packages was broken - the command could cause conflicts in the case of orphan packages; when we only do removals that won't lead to rebuilds, as is the case with autoremove without argument, we can actually just ignore orphans (with argument, only keep orphans within the reverse dep cone, as usual).
Instead of removing all unrelated packages, remove just the versions of related packages that we are guaranteed not to need.
|
Had we reached a conclusion that this won't work and that rebuilding opam 2.0 with GLPK 5 is the best we can do for now? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
WIP