[WIP] Cudf request preprocessing #4369
Merged
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
AltGr
force-pushed
the
cudf-preprocessing
branch
from
45ce811 to
d5b68ed
Compare
|
I've done some tests with this PR and here are my result: On a fresh
Test done on my normal work laptop carying an SSD, Intel i5-5200U and 16Go of RAM. This is very encouraging results, however not quite enough to get |
AltGr
force-pushed
the
cudf-preprocessing
branch
from
1faf7e4 to
8f66209
Compare
|
This ended up gathering quite some stuff; I could split it up, but it is basically just fixes we absolutely want on top of the Cudf preprocessing (with code dependencies); so that would involve either rewriting quite a bit, or waiting anyway for the preprocessing to be merged before filing more PRs.
|
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.
AltGr
force-pushed
the
cudf-preprocessing
branch
from
8f66209 to
20d92c6
Compare
This was
linked to
issues
Oct 16, 2020
Closed
rjbou
approved these changes
Oct 20, 2020
rjbou
reviewed
Oct 20, 2020
Co-authored-by: R. Boujbel <rjbou@ocamlpro.com>
# for free
to join this conversation on GitHub.
Already have an account?
# 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.
This adds a resolution of conflicts at low-level (directly on the CUDF
that is sent to the solver, but after the satisfiability check so that
it doesn't have an impact on the error messages).
It might be a better replacement for the high-level preprocessing that
tried to remove conflicting compilers in OpamSwitchState, when
computing the universe. Preliminary tests have shown it can be very
efficient for local switch creation cases where the OCaml version is
constrained.