Fix a space leak in package preferences (part of issue #3824). #3826
Conversation
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, thanks for your PR! By analyzing the annotation information on this pull request, we identified @kosmikus, @edsko and @BardurArantsson to be potential reviewers |
| -- 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` |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
|
Good catch and I agree the patch does what you say it does, and I'll believe you that it fixes the leak :-) Just a minor simplification suggestion. |
|
LGTM as well. |
|
Thanks! |
|
Thank you! (Someday we'll have regression tests for this sort of thing...) |
The space leak was introduced in #3594. #3594 added a new variable,
sortedVersions, to sort the subtrees under package choice nodes in the searchtree. However,
sortedVersionsreferenced the subtrees and caused them to beretained when it was not used. This commit forces evaluation of
sortedVersions.