Skip to content

[RFC] Add option to find best install plan before backjump limit #2917

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

Closed
wants to merge 2 commits into from

Conversation

grayjay
Copy link
Collaborator

@grayjay grayjay commented Nov 9, 2015

I've started implementing install plan scoring, as described in #2860. cabal install now prints the score of each install plan and allows the user to find better plans with the option --max-score, where zero is perfect. cabal should find the same install plans as before when no maximum is specified. This pull request isn't ready to be merged yet. It needs documentation and tests, and I have a few questions in TODO comments. Also, this is the most basic way to expose install plan scoring, and it might be better to automate determining when to continue searching before this is merged.

I rebased onto #2916 because this change made the existing space leak worse. Then I compared performance between #2916 and the latest commit. Memory usage is similar, but the latest commit is noticeably slower when the solver runs for longer. I compiled cabal with GHC 7.10.2 and ran cabal --ignore-sandbox install --dry-run --max-backjumps -1 with the following packages and compilers:

Package Compiler Measurement c1189c3 (#2916) 8689097 (#2917)
aeson GHC 7.10.2 time 0.884 s 0.876 s
hackage-server-0.4 GHC 7.10.2 time 6.01 s 6.66 s
hackage-server-0.4 GHC 7.6.3 time 122.6 s 140.5 s
hackage-server-0.4 GHC 7.6.3 total memory (+RTS -s) 256MB 258MB

These comments that I made on the issue are still relevant:
#2860 (comment)
#2860 (comment)

@grayjay
Copy link
Collaborator Author

grayjay commented Dec 18, 2015

I rebased this to deal with conflicts and made a few minor changes that shouldn't affect the behavior.

@grayjay grayjay force-pushed the optimal-build-plans-2 branch 2 times, most recently from d0d6592 to 32ca508 Compare December 22, 2015 19:32
@23Skidoo 23Skidoo changed the title Add option to specify minimum install plan quality [RFC] Add option to specify minimum install plan quality Jan 16, 2016
@23Skidoo
Copy link
Member

@kosmikus, would be nice if you could provide @grayjay with some feedback on this.

@BardurArantsson
Copy link
Collaborator

@grayjay Note that there are now new conflicts, probably because my dead code removal patch was merged.

@grayjay grayjay force-pushed the optimal-build-plans-2 branch from 32ca508 to 748e203 Compare January 17, 2016 21:53
@grayjay
Copy link
Collaborator Author

grayjay commented Jan 17, 2016

I resolved the conflicts. The dead code removal actually shortened the diff by about 80 lines.

@grayjay
Copy link
Collaborator Author

grayjay commented Jan 25, 2016

I tried exposing install plan scoring in a more user-friendly way. The latest commit adds a new option, --find-best-solution. The solver searches until the backjump limit and then uses the best-scoring install plan found. If there is no backjump limit, the search is exhaustive. The only benefit of --max-score now is that it can allow the solver to run faster by pruning low-scoring nodes during the whole run. --find-best-solution works by decreasing the maximum allowed score as the solver finds better and better solutions.

There are a few things I'd like to improve if this feature is added to cabal:

  • The solver is much slower than on master. This branch ran for 43% longer in one long-running test.
  • On master, explore creates a log from the search tree, and then logToProgress processes the log and applies the backjump limit. I think it would be easier to implement the interaction between --max-backjumps and --find-best-solution if I first moved application of the backjump limit into explore. That would take significant refactoring, though. This branch currently just stores the best install plans in the log entries, which is awkward.
  • I think --max-score is unnecessary if we have --find-best-solution, but I left it in for testing.
  • Currently none of the solver's log is shown without -v3. I think it would make sense to show some things at lower verbosity, such as the scores of solutions that are skipped, and whether the install plan that is used is the best or just the best before the backjump limit.

@grayjay grayjay changed the title [RFC] Add option to specify minimum install plan quality [RFC] Add option to find best install plan before backjump limit Jan 25, 2016
@grayjay grayjay force-pushed the optimal-build-plans-2 branch 2 times, most recently from fbb7188 to 9ada6d1 Compare April 27, 2016 06:23
@kosmikus
Copy link
Contributor

@grayjay Thanks for rebasing this. I'm hoping that I can finally take a look at this soon.

@grayjay grayjay force-pushed the optimal-build-plans-2 branch 3 times, most recently from b1dbe99 to e9b42ab Compare April 28, 2016 01:02
@grayjay
Copy link
Collaborator Author

grayjay commented Apr 28, 2016

@kosmikus Thanks. I just rebased on top of the module reorganization and added some comments and quickcheck tests. I also added a commit that does dynamic goal reordering based on conflict sets. It sometimes finds the best solution faster, especially when combined with --reorder-goals.

EDIT: --dynamic-goal-reordering can also prevent the solver from finding a solution quickly, so I don't know if it's any better on average.

@grayjay grayjay force-pushed the optimal-build-plans-2 branch 3 times, most recently from a6e621e to 4fdc5c2 Compare April 28, 2016 05:22
@kosmikus
Copy link
Contributor

@grayjay Thanks! I had just been experimenting with the same/a similar thing. My "dynamic-goal-reordering" branch is at https://github.com/kosmikus/cabal/tree/count-conflicts

My initial experiments with this have been very promising. There are only very few cases where it seems to be slower, and many where it seems to perform vastly better. So I'd actually be optimistic that something like this can become the new default.

I will have a look at how our approaches differ, if at all.

@grayjay
Copy link
Collaborator Author

grayjay commented Jul 8, 2016

@kosmikus I started rebasing this onto master, because I think it will be easier to split it apart when it is up to date.

@grayjay grayjay force-pushed the optimal-build-plans-2 branch from 0af7823 to 606b1a8 Compare July 15, 2016 05:33
@grayjay
Copy link
Collaborator Author

grayjay commented Jul 15, 2016

I'm done rebasing.

@grayjay
Copy link
Collaborator Author

grayjay commented Jul 23, 2016

I rebased onto #3594.

grayjay added a commit to grayjay/cabal that referenced this pull request Jul 25, 2016
@grayjay grayjay force-pushed the optimal-build-plans-2 branch from 21793a4 to 06fdb15 Compare July 31, 2016 05:36
@grayjay grayjay force-pushed the optimal-build-plans-2 branch from 06fdb15 to 7cc6f71 Compare September 4, 2016 04:35
grayjay added a commit to grayjay/cabal that referenced this pull request Sep 4, 2016
@ezyang ezyang modified the milestone: cabal-install 2.0 Sep 6, 2016
@grayjay grayjay force-pushed the optimal-build-plans-2 branch from 7cc6f71 to 7e7af59 Compare September 10, 2016 21:46
@grayjay grayjay force-pushed the optimal-build-plans-2 branch from 7e7af59 to 76dcea9 Compare September 11, 2016 04:10
@grayjay grayjay force-pushed the optimal-build-plans-2 branch 2 times, most recently from 16eff27 to 1321a7b Compare September 24, 2016 19:29
@grayjay grayjay force-pushed the optimal-build-plans-2 branch 5 times, most recently from fbabbac to 8362d04 Compare March 5, 2017 23:33
@grayjay grayjay force-pushed the optimal-build-plans-2 branch from 8362d04 to 3830b97 Compare March 6, 2017 01:33
@grayjay
Copy link
Collaborator Author

grayjay commented Jan 14, 2018

This PR needs more work.

@grayjay grayjay closed this Jan 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants