Commit 3b8cdbcb authored by kristenk's avatar kristenk
Browse files

Solver: Avoid removing goal choices from the tree when applying heuristics.

Previously, the solver applied goal-ordering heuristics by removing all
non-preferred goal choices when there was at least one preferred choice. This
left fewer goals for later steps, such as conflict counting, to work with.

This commit changes the heuristics so that they only sort the goals. I didn't
change the preferBaseGoalChoice heuristic, though, because it made the solver
slower in some cases. I also think that it is safe to always choose the version
of base first, because there is usually only one choice anyway.

I reversed the order of the heuristics, because sorting gives the most weight to
the last step, and pruning gives the most weight to the first step. The solver's
behavior should be unchanged with --no-count-conflicts.

Some notes on how I tested it:

I expected this change to improve performance in many cases by giving the
"conflict counting" heuristic more goals to choose from. However, I wasn't able
to find any cases where the solver made different choices compared with master,
except when I also used --reorder-goals. I spent a while looking, because it was
hard to believe.

I searched for examples first by comparing the verbose logs from this branch and
master for a few packages with many dependencies. Then I compared runtimes with
master when solving for each package on Hackage (with GHC 8.0.1), in order to
find packages with very large differences in runtime. Surprisingly, I didn't see
any where the change was over 50% in either direction. I reran ~10 with the
biggest difference and found two where the difference was more than noise. I
compared their logs, but they were also unchanged. Both were slower than master.
I profiled solving for grapefruit-ui-gtk, which was the slowest (18% slower than
master, with a runtime of ~20 seconds). I found that this branch spent about
twice as much time in Explore.getBestGoal. That makes sense, because getBestGoal
now has more goals to choose from. I didn't look into why the change had such a
big impact on that particular package.

I also tried running the solver on grapefruit-ui-gtk with --reorder-goals and no
backjump limit. This branch finished in about 67 seconds, and I stopped master
after ~8 minutes.

Then I compared runtime for another long-running combination of packages to test
the overhead of this change when the solver makes the same choices as master. I
ran 'cabal install --dry-run yesod phooey --max-backjumps -1' with GHC 8.0.1 and
took the average of 4 runs. master ran for 8.44 seconds, and this branch ran for
8.50 seconds, which is a difference of less than 1%.

Even though this change doesn't improve performance now, I think it's worthwhile
for simplifying the interactions between goal-ordering heuristics, and working
towards issue #3488 (Solver: Combine goal-ordering heuristics more effectively
by assigning scores to goal choices).
parent 62cee2ab
......@@ -8,6 +8,8 @@ module Distribution.Solver.Modular.PSQ
, length
, lookup
, filter
, filterIfAny
, filterIfAnyByKeys
, filterKeys
, firstOnly
, fromList
......@@ -122,23 +124,31 @@ minimumBy :: (a -> Int) -> PSQ k a -> PSQ k a
minimumBy sel (PSQ xs) =
PSQ [snd (S.minimumBy (comparing fst) (S.map (\ x -> (sel (snd x), x)) xs))]
-- | Sort the list so that values satisfying the predicate are first.
prefer :: (a -> Bool) -> PSQ k a -> PSQ k a
prefer p = sortBy $ flip (comparing p)
-- | Sort the list so that keys satisfying the predicate are first.
preferByKeys :: (k -> Bool) -> PSQ k a -> PSQ k a
preferByKeys p = sortByKeys $ flip (comparing p)
-- | Will partition the list according to the predicate. If
-- there is any element that satisfies the precidate, then only
-- the elements satisfying the predicate are returned.
-- Otherwise, the rest is returned.
--
prefer :: (a -> Bool) -> PSQ k a -> PSQ k a
prefer p (PSQ xs) =
filterIfAny :: (a -> Bool) -> PSQ k a -> PSQ k a
filterIfAny p (PSQ xs) =
let
(pro, con) = S.partition (p . snd) xs
in
if S.null pro then PSQ con else PSQ pro
-- | Variant of 'prefer' that takes a predicate on the keys
-- | Variant of 'filterIfAny' that takes a predicate on the keys
-- rather than on the values.
--
preferByKeys :: (k -> Bool) -> PSQ k a -> PSQ k a
preferByKeys p (PSQ xs) =
filterIfAnyByKeys :: (k -> Bool) -> PSQ k a -> PSQ k a
filterIfAnyByKeys p (PSQ xs) =
let
(pro, con) = S.partition (p . fst) xs
in
......
......@@ -323,12 +323,13 @@ firstGoal = trav go
-- Note that we keep empty choice nodes, because they mean success.
-- | Transformation that tries to make a decision on base as early as
-- possible. In nearly all cases, there's a single choice for the base
-- package. Also, fixing base early should lead to better error messages.
-- possible by pruning all other goals when base is available. In nearly
-- all cases, there's a single choice for the base package. Also, fixing
-- base early should lead to better error messages.
preferBaseGoalChoice :: Tree d c -> Tree d c
preferBaseGoalChoice = trav go
where
go (GoalChoiceF xs) = GoalChoiceF (P.preferByKeys isBase xs)
go (GoalChoiceF xs) = GoalChoiceF (P.filterIfAnyByKeys isBase xs)
go x = x
isBase :: Goal QPN -> Bool
......@@ -353,7 +354,7 @@ deferSetupChoices = trav go
deferWeakFlagChoices :: Tree d c -> Tree d c
deferWeakFlagChoices = trav go
where
go (GoalChoiceF xs) = GoalChoiceF (P.prefer noWeakStanza (P.prefer noWeakFlag xs))
go (GoalChoiceF xs) = GoalChoiceF (P.prefer noWeakFlag (P.prefer noWeakStanza xs))
go x = x
noWeakStanza :: Tree d c -> Bool
......@@ -398,7 +399,7 @@ preferEasyGoalChoices = trav go
preferReallyEasyGoalChoices :: Tree d c -> Tree d c
preferReallyEasyGoalChoices = trav go
where
go (GoalChoiceF xs) = GoalChoiceF (P.prefer zeroOrOneChoices xs)
go (GoalChoiceF xs) = GoalChoiceF (P.filterIfAny zeroOrOneChoices xs)
go x = x
-- | Monad used internally in enforceSingleInstanceRestriction
......
......@@ -115,8 +115,8 @@ solve sc cinfo idx pkgConfigDB userPrefs userConstraints userGoals =
in case goalOrder sc of
Nothing -> goalChoiceHeuristics .
heuristicsTree .
P.deferWeakFlagChoices .
P.deferSetupChoices .
P.deferWeakFlagChoices .
P.preferBaseGoalChoice
Just order -> P.firstGoal .
heuristicsTree .
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment