Skip to content

Fix a space leak in package preferences (part of issue #3824). #3826

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

Merged
merged 2 commits into from
Sep 12, 2016
Merged

Fix a space leak in package preferences (part of issue #3824). #3826

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d95d0e1
Fix a space leak in package preferences (part of issue #3824).
sebright Sep 12, 2016
2e5374e
Simplify Preference.addWeights.
sebright Sep 12, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 13 additions & 4 deletions cabal-install/Distribution/Solver/Modular/Preference.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,17 +51,26 @@ import qualified Distribution.Solver.Modular.WeightedPSQ as W
-- | Update the weights of children under 'PChoice' nodes. 'addWeights' takes a
-- list of weight-calculating functions in order to avoid sorting the package
-- choices multiple times. Each function takes the package name, sorted list of
-- siblings' versions, and package option. 'addWeights' prepends the new
-- children's versions, and package option. 'addWeights' prepends the new
-- weights to the existing weights, which gives precedence to preferences that
-- are applied later.
addWeights :: [PN -> [Ver] -> POption -> Weight] -> Tree a -> Tree a
addWeights fs = trav go
where
go :: TreeF a (Tree a) -> TreeF a (Tree a)
go (PChoiceF qpn@(Q _ pn) x cs) =
let sortedVersions = L.sortBy (flip compare) $ L.map version (W.keys cs)
let versions = L.map version (W.keys cs)
sortedVersions = L.sortBy (flip compare) versions
weights k = [f pn sortedVersions k | f <- fs]
in PChoiceF qpn x $
W.mapWeightsWithKey (\k w -> weights k ++ w) cs

elemsToWhnf :: [a] -> ()
elemsToWhnf = foldr seq ()
in PChoiceF qpn x
-- Evaluate the children's versions before evaluating any of the
-- subtrees, so that 'versions' doesn't hold onto all of the
-- subtrees (referenced by cs) and cause a space leak.
(elemsToWhnf versions `seq`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess you can simplify this a little by forcing the sortedVersions and thus not having to define versions separately from sortedVersions.

Once versions are forced, then evaluating the result of the list sort doesn't force or retain any more, it's just a rearrangement of the (already forced) versions.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had forced versions separately to avoid sorting the list unnecessarily. I don't think that was needed, though, because sortedVersions is almost always evaluated eventually (which is probably why this bug didn't show up sooner). I applied your suggestion.

W.mapWeightsWithKey (\k w -> weights k ++ w) cs)
go x = x

addWeight :: (PN -> [Ver] -> POption -> Weight) -> Tree a -> Tree a
Expand Down