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

Speedup installation when the package has no conflict #4658

Closed
wants to merge 1 commit into from

Conversation

kit-ty-kate
Copy link
Member

e.g. when calling opam install utop on a 4.12 switch, opam spends 4s in Dose3 before calling the actual solver

Partly fixes an issue detected by @talex5 in ocurrent/docker-base-images#102 (comment)

e.g. when calling `opam install utop` on a 4.12 switch, opam spends 4s in Dose3 before calling the actual solver
@kit-ty-kate kit-ty-kate requested a review from AltGr May 11, 2021 15:50
@kit-ty-kate kit-ty-kate added this to the 2.1.1 milestone May 11, 2021
@AltGr
Copy link
Member

AltGr commented May 18, 2021

This change would prevent all conflict messages from being printed, replacing them by "solver error": it's not a good idea as is.

The other option would be to revert to what we had in 2.0: first call the external solver, then call check_request (the dose3 SAT) to get the conflict information.

The change was made for a reason though, so we need to examine the balance here. My impression is that the order in 2.1 is better for the user in most cases, but here again, I hadn't hit such cases where check_request had a significant cost as of now.

  • with the 2.0 order (external then check_request):
    • in case of success, you only get the (generally long, in comparison) external solver time without additional penalty
    • in case of conflict, you get a (generally quite long) external solving time before it realises there is no solution, before we run check_request to (quickly) get the conflicts
  • with the current 2.1 order (check_request first):
    • in case of success, you always get the penalty of the added check_request call. In general it is negligible compared to the solving time, but that may be different in problematic cases and with the 0install solver, as demonstrated in thereferenced issue.
    • in case of conflict, you detect it right away and print the details without calling the external solver at all

In a perfect world we would call both in parallel :)

@rjbou rjbou added the PR: WIP Not for merge at this stage label Sep 17, 2021
@dra27 dra27 modified the milestones: 2.1.1, 2.1.2 Sep 17, 2021
AltGr added a commit to OCamlPro/opam that referenced this pull request Oct 28, 2021
Closes ocaml#4658, see the discussion there.

It'd probably be worth it to check the Dose3 check times though; they
may be due to the increased complexity of the repo, but also maybe to
the new dose3 version ?

Also I didn't re-check the response times of the different solvers in
case of complex, unsat cases : the worrying case here was mccs stalling
for tenths of seconds, even sometimes timing out, while dose3 (which we
would call anyway) would detect the conflict in a fraction of a second.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: WIP Not for merge at this stage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants