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.

Sign up for GitHub

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

Jump to bottom

[opam 2.1~alpha3] opam list --installable is really slow #4311

Closed
kit-ty-kate opened this issue Aug 12, 2020 · 10 comments · Fixed by #5024
Closed

[opam 2.1~alpha3] opam list --installable is really slow #4311

kit-ty-kate opened this issue Aug 12, 2020 · 10 comments · Fixed by #5024
Assignees
@AltGr
Labels
AREA: SOLVER
Projects
Opam 2.2.0 Opam 2.1.x
Milestone
2.1.3

Comments

@kit-ty-kate
Copy link
Member

With opam 2.1~alpha3:

opam@5ab5892e7927:~$ time opam-2.1 list --installable --depends-on ppx_tools
# Packages matching: installable & depends-on(ppx_tools)
[...]
real	3m53.765s
user	3m52.204s
sys	0m0.683s

With opam 2.0.7:

opam@5ab5892e7927:~$ time opam list --installable --depends-on ppx_tools
# Packages matching: installable & depends-on(ppx_tools)
[...]
real	0m20.094s
user	0m19.603s
sys	0m0.206s

You can use docker run --rm -it ocurrent/opam:debian-10-ocaml-4.10 to reproduce this.
@rjbou rjbou added this to To do in Opam 2.1.x via automation Aug 17, 2020
@rjbou rjbou added this to the 2.1.0~beta milestone Aug 17, 2020
@rjbou
Copy link
Collaborator

rjbou commented Aug 20, 2020

The bottleneck is in universe check. @AltGr, an idea?

@AltGr
Copy link
Member

AltGr commented Aug 21, 2020

... which is the actual installability check 😬

the explanation can only be in the differences in how the universe is encoded, I'll check but there might be no easy way out.

@rjbou rjbou modified the milestones: 2.1.0~beta, 2.1.0 Aug 26, 2020
@kit-ty-kate
Copy link
Member Author

Just a follow-up, here is the same command when the switch invariant is not the default one when upgrading from 2.0:

$ opam-2.1 switch invariant
["ocaml-base-compiler" {= "4.11.1"}]
$ time opam-2.1 list --installable --depends-on ppx_tools
[...]
real	8m24.987s
user	8m24.435s
sys	0m0.453s

(it seems to be getting worse? 😟)

@emillon
Copy link
Contributor

emillon commented Jan 21, 2021

I bisected this and this has been introduced in #3894 (51e3521 is good, 8ca4d4c is bad, couldn't test the 6 commits in between).

@Armael
Copy link
Member

Armael commented Jan 23, 2021

FWIW I'm hitting the same issue; an opam list --installable on a 4.09.0 switch took 5 minutes where it used to take only a few seconds.

@kit-ty-kate
Copy link
Member Author

Another example, using opam-repository at fb801efd13b2e6370271608f2bfd528f2eea72bb, the following command:

opam list -s --color=never --depends-on minios-xen.0.7 --coinstallable-with minios-xen.0.7 --installable --all-versions --recursive --depopts

takes:

  • 1m27s with opam 2.0.7
  • 19m43s with opam 2.1.0~beta4

@avsm avsm modified the milestones: 2.1.0, 2.1.0~rc Feb 19, 2021
@avsm
Copy link
Member

avsm commented Feb 19, 2021

dev meeting: this one's pretty important, but it could go in after rc as its quite a targetted fix. Not completely blocking 2.1.0 but would be nice to fix before 2.1.1

@rjbou rjbou modified the milestones: 2.1.0, 2.1.1 Feb 26, 2021
@dra27 dra27 removed this from To do in Opam 2.1.x Mar 18, 2021
@kit-ty-kate
Copy link
Member Author

kit-ty-kate commented Apr 26, 2021 •
edited

Interestingly enough

opam list -s --color=never --depends-on 'ocsigenserver.3.0.0' --coinstallable-with 'ocsigenserver.3.0.0' --installable --all-versions --recursive --depopts

with ocaml/opam-repository@0b5927e, takes more than 6 minutes to tell me

[ERROR] Package ocsigenserver has no version 3.0.0.

@dra27 dra27 added this to To do in Opam 2.2.0 via automation Jul 8, 2021
@rjbou rjbou added this to To do in Opam 2.1.x Jul 30, 2021
@dra27 dra27 modified the milestones: 2.1.1, 2.1.2 Sep 17, 2021
@AltGr AltGr mentioned this issue Nov 2, 2021
Merged
@cmovcc cmovcc mentioned this issue Nov 17, 2021
Closed
@dra27 dra27 modified the milestones: 2.1.2, 2.1.3 Nov 19, 2021
@kit-ty-kate
Copy link
Member Author

As far as I can see, this issue was fixed by #4882 and seems even faster than before.

Opam 2.1.x automation moved this from To do to Done Jan 16, 2022
Opam 2.2.0 automation moved this from To do to Done Jan 16, 2022
@Armael
Copy link
Member

Armael commented Jan 16, 2022

Nice! Thanks a lot for the fix and the update.

AltGr added a commit to AltGr/opam that referenced this issue Jan 25, 2022
@AltGr
Add pre-processing to coinstallability checks
01d23da 
Addresses some remaining costly cases in ocaml#4311

The patch includes a small reorganisation of `OpamSolver`, but the general idea
is to fix the performance regression compared to 2.0:

- with the introduction of solver invariants, the pre-processing that trimmed
  packages conflicting with the base in `OpamState` was removed
- it was replaced by something much more general (and reliable) at the
  `OpamCudf` level
- but only for calls to the external solver, until now

NOTE: this enforces the invariant even for `opam install --coinstallable-with`,
which is consistent with 2.0 but had changed in 2.1. Without it we can't really
expect reasonable performance in general anyway.
@AltGr AltGr mentioned this issue Jan 25, 2022
Merged
AltGr added a commit to AltGr/opam that referenced this issue Jan 26, 2022
@AltGr
Add pre-processing to coinstallability checks
cd67e20 
Addresses some remaining costly cases in ocaml#4311

The patch includes a small reorganisation of `OpamSolver`, but the general idea
is to fix the performance regression compared to 2.0:

- with the introduction of solver invariants, the pre-processing that trimmed
  packages conflicting with the base in `OpamState` was removed
- it was replaced by something much more general (and reliable) at the
  `OpamCudf` level
- but only for calls to the external solver, until now

NOTE: this enforces the invariant even for `opam install --coinstallable-with`,
which is consistent with 2.0 but had changed in 2.1. Without it we can't really
expect reasonable performance in general anyway.
AltGr added a commit to AltGr/opam that referenced this issue Feb 2, 2022
@AltGr
Add pre-processing to coinstallability checks
f4fc865 
Addresses some remaining costly cases in ocaml#4311

The patch includes a small reorganisation of `OpamSolver`, but the general idea
is to fix the performance regression compared to 2.0:

- with the introduction of solver invariants, the pre-processing that trimmed
  packages conflicting with the base in `OpamState` was removed
- it was replaced by something much more general (and reliable) at the
  `OpamCudf` level
- but only for calls to the external solver, until now

NOTE: this enforces the invariant even for `opam install --coinstallable-with`,
which is consistent with 2.0 but had changed in 2.1. Without it we can't really
expect reasonable performance in general anyway.
@rjbou rjbou linked a pull request Feb 2, 2022 that will close this issue
Add pre-processing to coinstallability checks #5024
Merged
AltGr added a commit to OCamlPro/opam that referenced this issue Feb 4, 2022
@AltGr
Add pre-processing to coinstallability checks
0d3892a 
Addresses some remaining costly cases in ocaml#4311

The patch includes a small reorganisation of `OpamSolver`, but the general idea
is to fix the performance regression compared to 2.0:

- with the introduction of solver invariants, the pre-processing that trimmed
  packages conflicting with the base in `OpamState` was removed
- it was replaced by something much more general (and reliable) at the
  `OpamCudf` level
- but only for calls to the external solver, until now

NOTE: this enforces the invariant even for `opam install --coinstallable-with`,
which is consistent with 2.0 but had changed in 2.1. Without it we can't really
expect reasonable performance in general anyway.
rjbou pushed a commit to OCamlPro/opam that referenced this issue Feb 18, 2022
@AltGr @rjbou
Add pre-processing to coinstallability checks
5cbc593 
Addresses some remaining costly cases in ocaml#4311

The patch includes a small reorganisation of `OpamSolver`, but the general idea
is to fix the performance regression compared to 2.0:

- with the introduction of solver invariants, the pre-processing that trimmed
  packages conflicting with the base in `OpamState` was removed
- it was replaced by something much more general (and reliable) at the
  `OpamCudf` level
- but only for calls to the external solver, until now

NOTE: this enforces the invariant even for `opam install --coinstallable-with`,
which is consistent with 2.0 but had changed in 2.1. Without it we can't really
expect reasonable performance in general anyway.
rjbou pushed a commit to AltGr/opam that referenced this issue Feb 22, 2022
@AltGr @rjbou
Add pre-processing to coinstallability checks
2d53f47 
Addresses some remaining costly cases in ocaml#4311

The patch includes a small reorganisation of `OpamSolver`, but the general idea
is to fix the performance regression compared to 2.0:

- with the introduction of solver invariants, the pre-processing that trimmed
  packages conflicting with the base in `OpamState` was removed
- it was replaced by something much more general (and reliable) at the
  `OpamCudf` level
- but only for calls to the external solver, until now

NOTE: this enforces the invariant even for `opam install --coinstallable-with`,
which is consistent with 2.0 but had changed in 2.1. Without it we can't really
expect reasonable performance in general anyway.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@AltGr AltGr

Labels
AREA: SOLVER
Projects
Opam 2.1.x
  
Done
Opam 2.2.0
  
Done
Milestone
2.1.3
Development

Successfully merging a pull request may close this issue.

Add pre-processing to coinstallability checks AltGr/opam
7 participants
@avsm @Armael @emillon @AltGr @kit-ty-kate @dra27 @rjbou