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

Improve goal reorder heuristics. #3208

Merged
merged 1 commit into from Mar 4, 2016

Conversation

Projects
None yet
3 participants
@kosmikus
Copy link
Contributor

kosmikus commented Mar 3, 2016

This change primarily does two things:

  1. For --reorder-goals, we use a dedicated datatype Degree
    rather than an Int to compute the approximate branching
    degree. We map 0 and 1 to the same value. We then use a
    lazy ordering and a shortcutting minimum function to look
    for the "best" goal.

    The motivation here is that we do not want to spend
    unnecessary work. Following any goal that has 0 or 1 as degree
    cannot really be "wrong", so we should not look at any others
    and waste time.

    This will still not always make the use of --reorder-goals
    better than not using it, but it will reduce the overhead
    introduced by it.

  2. We use partitioning rather than sorting for most of the other
    goal reordering heuristics that are active in all situations.
    I think this is slightly more straightforward and also slightly
    more efficient, whether --reorder-goals is used or not.

I have run some preliminary performance comparisons and they
seem to confirm that in both cases separately (with or without
--reorder-goals), these changes are a relative improvement
over the status quo. I will run additional tests before
merging this into master.

Improve goal reorder heuristics.
This change primarily does two things:

1. For `--reorder-goals`, we use a dedicated datatype `Degree`
   rather than an `Int` to compute the approximate branching
   degree. We map 0 and 1 to the same value. We then use a
   lazy ordering and a shortcutting minimum function to look
   for the "best" goal.

   The motivation here is that we do not want to spend
   unnecessary work. Following any goal that has 0 or 1 as degree
   cannot really be "wrong", so we should not look at any others
   and waste time.

   This will still not always make the use of `--reorder-goals`
   better than not using it, but it will reduce the overhead
   introduced by it.

2. We use partitioning rather than sorting for most of the other
   goal reordering heuristics that are active in all situations.
   I think this is slightly more straightforward and also slightly
   more efficient, whether `--reorder-goals` is used or not.

I have run some preliminary performance comparisons and they
seem to confirm that in both cases separately (with or without
`--reorder-goals`), these changes are a relative improvement
over the status quo. I will run additional tests before
merging this into master.
@kosmikus

This comment has been minimized.

Copy link
Contributor

kosmikus commented Mar 3, 2016

/cc @hvr

@hvr

This comment has been minimized.

Copy link
Member

hvr commented Mar 3, 2016

I've ran a few random tests for cases which stress the solver a bit (solving a full stackage-set, trying to find (when there isn't any) install plans for packages for GHC8, etc..), and they all show a significant improvement of --reorder-goals. This looks very promising!

@kosmikus

This comment has been minimized.

Copy link
Contributor

kosmikus commented Mar 4, 2016

@23Skidoo After running some tests, it still looks like this is a clear improvement, and I haven't found any result deviations either. Are you ok if I merge this? I have no idea what your policies for 1.24 are, but I wouldn't mind having it go in that release as well.

@23Skidoo

This comment has been minimized.

Copy link
Member

23Skidoo commented Mar 4, 2016

@kosmikus Sure, please do. It'd be also nice to include #2916 and #2914 in 1.24.

kosmikus added a commit that referenced this pull request Mar 4, 2016

Merge pull request #3208 from kosmikus/new-lazy-reorder-goals
Improve goal reorder heuristics.

@kosmikus kosmikus merged commit 607f25d into haskell:master Mar 4, 2016

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@kosmikus

This comment has been minimized.

Copy link
Contributor

kosmikus commented Mar 4, 2016

@23Skidoo Thanks. And agreed about #2916 and #2914.

@23Skidoo

This comment has been minimized.

Copy link
Member

23Skidoo commented Mar 4, 2016

Cherry-picked into the 1.24 branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment