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

Cabal can't new-build bytestring with GHC 8.0.1 #3824

Open
blamario opened this issue Sep 11, 2016 · 12 comments
Open

Cabal can't new-build bytestring with GHC 8.0.1 #3824

blamario opened this issue Sep 11, 2016 · 12 comments

Comments

@blamario
Copy link
Contributor

~/Haskell/bytestring$ cd ~/tmp/
~/tmp$ ghc --version
The Glorious Glasgow Haskell Compilation System, version 8.0.1
~/tmp$ cabal --version
cabal-install version 1.24.0.0
compiled using version 1.24.0.0 of the Cabal library
~/tmp$ date
Sun Sep 11 15:15:40 EDT 2016
~/tmp$ git clone https://github.com/haskell/bytestring.git
Cloning into 'bytestring'...
remote: Counting objects: 6795, done.
remote: Total 6795 (delta 0), reused 0 (delta 0), pack-reused 6795
Receiving objects: 100% (6795/6795), 12.25 MiB | 3.33 MiB/s, done.
Resolving deltas: 100% (4409/4409), done.
Checking connectivity... done.
~/tmp$ cd bytestring/

Starting with a clean slate, cabal-install-1.24 ponders for 83 seconds, then claims that the globally installed unix-2.7.2.0 package is broken:

~/tmp/bytestring$ mv ~/.cabal/ ~/.ghc/ ~/tmp/
~/tmp/bytestring$ ~/tmp/.cabal/bin/cabal update
Downloading the latest package list from hackage.haskell.org
~/tmp/bytestring$ /usr/bin/time ~/tmp/.cabal/bin/cabal new-build
Resolving dependencies...
cabal: Could not resolve dependencies:
trying: process-1.4.2.0/installed-1.4... (dependency of
optparse-applicative-0.12.1.0)
next goal: unix (dependency of process-1.4.2.0/installed-1.4...)
rejecting: unix-2.7.2.0/installed-2.7... (package is broken)
rejecting: unix-2.7.2.0, unix-2.7.1.0, unix-2.7.0.1, unix-2.7.0.0,
unix-2.6.0.1, unix-2.6.0.0, unix-2.5.1.1, unix-2.5.1.0, unix-2.5.0.0,
unix-2.4.2.0, unix-2.4.1.0, unix-2.4.0.2, unix-2.4.0.1, unix-2.4.0.0,
unix-2.3.2.0, unix-2.3.1.0, unix-2.3.0.0, unix-2.2.0.0, unix-2.0 (conflict:
process => unix==2.7.2.0/installed-2.7...)
Backjump limit reached (currently 2000, change with --max-backjumps or try to
run with --reorder-goals).
Command exited with non-zero status 1
83.05user 0.16system 1:23.74elapsed 99%CPU (0avgtext+0avgdata 298340maxresident)k
20432inputs+8outputs (75major+78990minor)pagefaults 0swaps

The HEAD version of cabal-install instead quickly diverges and thrashes my machine, unless I stop it:

~/tmp/bytestring$ /usr/bin/time ~/Haskell/cabal/dist-newstyle/build/cabal-install-1.25.0.0/build/cabal/cabal new-build
Resolving dependencies...
^CCommand terminated by signal 2
23.38user 1.23system 0:24.63elapsed 99%CPU (0avgtext+0avgdata 6580484maxresident)k
0inputs+8outputs (0major+1649612minor)pagefaults 0swaps

I tried installing a fresh new user-local unix-2.7.2.0 but that didn't help.

@grayjay grayjay self-assigned this Sep 11, 2016
@23Skidoo
Copy link
Member

ponders for 83 seconds, then claims that the globally installed unix-2.7.2.0 package is broken:

Can reproduce with a GHC 8.0.2 snapshot and a recent cabal-install from master.

@ezyang
Copy link
Contributor

ezyang commented Sep 11, 2016

Is the problem possibly that new-build will always force use of an installed package if it's available, but since you have a new bytestring we can't use the old unix, so it's unsat? I don't actually know where this policy gets applied.

@ezyang
Copy link
Contributor

ezyang commented Sep 11, 2016

Definitely feels like a blocker for 2.0. @grayjay if you don't have time to investigate feel free to unassign yourself.

@grayjay
Copy link
Collaborator

grayjay commented Sep 11, 2016

Okay. I at least wanted to look into the last problem.

@grayjay
Copy link
Collaborator

grayjay commented Sep 11, 2016

The space leak was introduced in #3594 and is unrelated to the broken package. I'll work on a PR.

@23Skidoo
Copy link
Member

/cc @dcoutts

grayjay added a commit to grayjay/cabal that referenced this issue Sep 12, 2016
The space leak was introduced in haskell#3594. haskell#3594 added a new variable,
sortedVersions, to sort the subtrees under package choice nodes in the search
tree. However, sortedVersions referenced the subtrees and caused them to be
retained when it was not used. This commit forces evaluation of
sortedVersions.
@grayjay
Copy link
Collaborator

grayjay commented Sep 12, 2016

The space leak was actually caused by #2899 and a bug in #3594. I fixed the newer issue in #3826, but #2899 will take more work.

@grayjay grayjay removed their assignment Sep 12, 2016
grayjay added a commit that referenced this issue Sep 12, 2016
Fix a space leak in package preferences (part of issue #3824).
@grayjay
Copy link
Collaborator

grayjay commented Oct 29, 2016

I did some more debugging, and I think there are multiple issues here. The reason that the solver didn't find a solution is that new-build first tries to enable tests, but the tests create a cyclic dependency. A bug in the solver prevents it from falling back to building without tests. The solver also takes too long to find that there is no solution, so it hits the backjump limit. It finds a solution immediately with --disable-tests.

  1. The message about the installed unix being broken isn't the main error, it's just the first error. That is issue Incorrect error messages for non-existing dependencies #941.

  2. I can reproduce the unix error with cabal-install 1.22.9.0, so it's not specific to new-build. The solver can't find the installed version of bytestring, which has the same version as the source, and the installed unix depends on it. I don't think the error could prevent cabal from finding a solution in this case, though, because any install plan involving the installed and source versions of bytestring would be pruned when the solver enforced the Single Instance Restriction.

  3. The solver is slow to backtrack in the presence of cyclic dependencies, because it only checks for cycles once it has a complete install plan. We should probably check after every step and possibly use incremental cycle detection on the graph for better performance.

  4. The conflict set for cyclic dependencies isn't correct. The solver currently checks for cycles on a Done node by looking for a strongly connected component of size > 1. Then it uses the packages in the strongly connected component as the conflict set. However, it isn't just the packages that create the cycle; flag/test/bench choices can control whether the packages actually depend on each other, so we should include them, too.

  5. The conflict set also contains unnecessary variables, which could make backjumping less effective. We only need one cycle, but the strongly connected component can contain many, especially after a choice like enabling testing introduces a lot of dependencies. It would be nice to only use the shortest cycle.

    In this example, the first cyclic dependency failure caused this error when I added cycle checking after every step: fail (cyclic dependencies; conflict set: ansi-terminal, ansi-wl-pprint, binary, bytestring, directory, test-framework, test-framework-hunit, test-framework-quickcheck2, text, unix, xml)

    A better one would be bytestring, bytestring-0.10.8.1:*test, test-framework, xml, because bytestring's test suites depend on test-framework, test-framework depends on xml, and xml depends on bytestring.

We would need something like #3422 in order for cabal to find a solution with tests enabled. Without that, I think we would at least need to fix 3 and 4 so that cabal can fail more quickly. 3 doesn't seem too hard, unless performance is a problem. I'm not sure how to fix 4.

@ezyang
Copy link
Contributor

ezyang commented Oct 31, 2016

Thank you for the detailed analysis. Does (4) become less of a problem if we "componentize" the solver more?

@grayjay
Copy link
Collaborator

grayjay commented Nov 4, 2016

Depending on how we implement component-based solving, it could help with the missing test/bench variables, though it probably wouldn't help with flags.

@grayjay
Copy link
Collaborator

grayjay commented Nov 5, 2016

I just noticed that #1185 describes the broken package issue.

grayjay added a commit to grayjay/cabal that referenced this issue Dec 19, 2016
Previously, the solver only checked for cycles after it had already found a
solution. That reduced the number of times that it performed the check in the
common case when there were no cycles. However, when there was a cycle, the
solver could spend a lot of time searching subtrees that already had a cyclic
dependency and therefore could not lead to a solution. This is part of
haskell#3824.

Changes in this commit:
- Store the reverse dependency map on all choice nodes in the search tree, so
  that 'detectCyclesPhase' can access it at every step.
- Check for cycles incrementally at every step. Any new cycle must contain the
  current package, so we just check whether the current package is reachable
  from its neighbors.
- If there is a cycle, we convert the map to a graph and find a strongly
  connected component, as before.
- Instead of using the whole strongly connected component as the conflict set,
  we select one cycle. Smaller conflict sets are better for backjumping.
- The incremental cycle detection automatically fixes a bug where the solver
  filtered out the message about cyclic dependencies when it summarized the full
  log. The bug occurred when the failure message was not immediately after the
  line where the solver chose one of the packages involved in the conflict. See
  haskell#4154.

I tried several approaches before I found something with reasonable performance.
Here is a comparison of runtime and memory usage. I turned off assertions when
building cabal.

Index state: index-state(hackage.haskell.org) = 2016-12-03T17:22:05Z
GHC 8.0.1

Runtime in seconds:
Packages                    Search tree depth   Trials   master   This PR   haskell#1      haskell#2
yesod                       343                 3        2.00     2.00      2.13    2.02
yesod gi-glib leksah        744                 3        3.21     3.31      4.10    3.48
phooey                      66                  3        3.48     3.54      3.56    3.57
stackage nightly snapshot   6791                1        186      193       357     191

Total memory usage in MB, with '+RTS -s':
Packages                                        Trials   master    This PR   haskell#1     haskell#2
yesod                                           1         189       188       188     198
yesod gi-glib leksah                            1         257       257       263     306
stackage nightly snapshot                       1        1288      1338      1432   12699

haskell#1 - Same as master, but with cycle checking (Data.Graph.stronglyConnComp) after
     every step.
haskell#2 - Store dependencies in Distribution.Compat.Graph in the search tree, and
     check for cycles containing the current package at every step.
grayjay added a commit to grayjay/cabal that referenced this issue Dec 19, 2016
Previously, the solver only checked for cycles after it had already found a
solution. That reduced the number of times that it performed the check in the
common case where there were no cycles. However, when there was a cycle, the
solver could spend a lot of time searching subtrees that already had a cyclic
dependency and therefore could not lead to a solution. This is part of
haskell#3824.

Changes in this commit:
- Store the reverse dependency map on all choice nodes in the search tree, so
  that 'detectCyclesPhase' can access it at every step.
- Check for cycles incrementally at every step. Any new cycle must contain the
  current package, so we just check whether the current package is reachable
  from its neighbors.
- If there is a cycle, we convert the map to a graph and find a strongly
  connected component, as before.
- Instead of using the whole strongly connected component as the conflict set,
  we select one cycle. Smaller conflict sets are better for backjumping.
- The incremental cycle detection automatically fixes a bug where the solver
  filtered out the message about cyclic dependencies when it summarized the full
  log. The bug occurred when the failure message was not immediately after the
  line where the solver chose one of the packages involved in the conflict. See
  haskell#4154.

I tried several approaches and compared performance when solving for
packages with different numbers of dependencies. Here are the results. None of
these runs involved any cycles, so they should have only tested the overhead of
cycle checking. I turned off assertions when building cabal.

Index state: index-state(hackage.haskell.org) = 2016-12-03T17:22:05Z
GHC 8.0.1

Runtime in seconds:
Packages                    Search tree depth   Trials   master   This PR   haskell#1      haskell#2
yesod                       343                 3        2.00     2.00      2.13    2.02
yesod gi-glib leksah        744                 3        3.21     3.31      4.10    3.48
phooey                      66                  3        3.48     3.54      3.56    3.57
Stackage nightly snapshot   6791                1        186      193       357     191

Total memory usage in MB, with '+RTS -s':
Packages                                        Trials   master    This PR   haskell#1     haskell#2
yesod                                           1         189       188       188     198
yesod gi-glib leksah                            1         257       257       263     306
Stackage nightly snapshot                       1        1288      1338      1432   12699

haskell#1 - Same as master, but with cycle checking (Data.Graph.stronglyConnComp) after
     every step.
haskell#2 - Store dependencies in Distribution.Compat.Graph in the search tree, and
     check for cycles containing the current package at every step.
ezyang pushed a commit that referenced this issue Dec 27, 2016
Previously, the solver only checked for cycles after it had already found a
solution. That reduced the number of times that it performed the check in the
common case where there were no cycles. However, when there was a cycle, the
solver could spend a lot of time searching subtrees that already had a cyclic
dependency and therefore could not lead to a solution. This is part of
#3824.

Changes in this commit:
- Store the reverse dependency map on all choice nodes in the search tree, so
  that 'detectCyclesPhase' can access it at every step.
- Check for cycles incrementally at every step. Any new cycle must contain the
  current package, so we just check whether the current package is reachable
  from its neighbors.
- If there is a cycle, we convert the map to a graph and find a strongly
  connected component, as before.
- Instead of using the whole strongly connected component as the conflict set,
  we select one cycle. Smaller conflict sets are better for backjumping.
- The incremental cycle detection automatically fixes a bug where the solver
  filtered out the message about cyclic dependencies when it summarized the full
  log. The bug occurred when the failure message was not immediately after the
  line where the solver chose one of the packages involved in the conflict. See
  #4154.

I tried several approaches and compared performance when solving for
packages with different numbers of dependencies. Here are the results. None of
these runs involved any cycles, so they should have only tested the overhead of
cycle checking. I turned off assertions when building cabal.

Index state: index-state(hackage.haskell.org) = 2016-12-03T17:22:05Z
GHC 8.0.1

Runtime in seconds:
Packages                    Search tree depth   Trials   master   This PR   #1      #2
yesod                       343                 3        2.00     2.00      2.13    2.02
yesod gi-glib leksah        744                 3        3.21     3.31      4.10    3.48
phooey                      66                  3        3.48     3.54      3.56    3.57
Stackage nightly snapshot   6791                1        186      193       357     191

Total memory usage in MB, with '+RTS -s':
Packages                                        Trials   master    This PR   #1     #2
yesod                                           1         189       188       188     198
yesod gi-glib leksah                            1         257       257       263     306
Stackage nightly snapshot                       1        1288      1338      1432   12699

#1 - Same as master, but with cycle checking (Data.Graph.stronglyConnComp) after
     every step.
#2 - Store dependencies in Distribution.Compat.Graph in the search tree, and
     check for cycles containing the current package at every step.
@23Skidoo 23Skidoo modified the milestones: 2.0.1, 2.0 Feb 17, 2017
@grayjay
Copy link
Collaborator

grayjay commented Jun 27, 2017

#4176 fixed some of the issues with cyclic dependencies, so I think that the only remaining issues are #941, #1185, #1575, and flag/stanza variables missing from conflict sets for cyclic dependencies (item 4 in my previous comment: #3824 (comment)).

@23Skidoo 23Skidoo removed this from the 2.0.1 milestone Sep 19, 2017
@23Skidoo 23Skidoo added this to the 2.0.2 milestone Sep 19, 2017
@23Skidoo 23Skidoo modified the milestones: 2.0.2, 2.4 Aug 29, 2018
@23Skidoo 23Skidoo modified the milestones: 2.4, 2.4.1 Sep 17, 2018
@23Skidoo 23Skidoo modified the milestones: 2.4.1.0, 2.4.2.0 Apr 26, 2019
@phadej phadej modified the milestones: 2.4.2.0, 3.4 Nov 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants