Skip to content
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

Unzip Map and IntMap more strictly #163

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

treeowl
Copy link

@treeowl treeowl commented Aug 22, 2021

The previous Map (same for IntMap throughout) instance
would first map eagerly over tha Map, producing an entire
Map full of thunks to apply the unzipWith function. Then
it would build two more entire Maps full of thunks to select
components of each pair. We can do it eagerly using
Data.Biapplicative.traverseBia and a simple pair bifunctor.

The previous `Map` (same for `IntMap` throughout) instance
would first map eagerly over tha `Map`, producing an entire
`Map` full of thunks to apply the `unzipWith` function. Then
it would build two more entire `Map`s full of thunks to select
components of each pair. We can do it eagerly using
`Data.Biapplicative.traverseBia` and a simple pair bifunctor.
@treeowl
Copy link
Author

treeowl commented Aug 22, 2021

Note: if you really want to be super-lazy, we can at least avoid three entire Maps and needing to worry about whether the resulting Maps even contain selector thunks (or if they can leak space).

unzipWith f = unSBPair . traverseBia (SBPair . blah)
  where
    blah c = let
      {-# NOINLINE fc #-} -- make sure the result of f c is shared
      {-# NOINLINE a #-} -- make sure we get selector thunks
      {-# NOINLINE b #-}
      fc = f c
      (a, b) = fc
    in (a, b)

treeowl added a commit to treeowl/these that referenced this pull request Aug 22, 2021
The previous `Map` (same for `IntMap` throughout) instance
would first map eagerly over tha `Map`, producing an entire
`Map` full of thunks to apply the `unzipWith` function. Then
it would build two more entire `Map`s full of thunks to select
components of each pair. Depending on inlining and such, the
resulting maps may or may not have contained selector thunks;
if not, they could leak memory. Fix that.

NOTE: This PR is an alternative to haskellari#163. This one preserves
the precise laziness properties of the previous implementation.
@treeowl
Copy link
Author

treeowl commented Aug 22, 2021

I've opened #164 with a version that preserves the precise laziness of the current version but without the leak potential and without building a third map structure.

@treeowl
Copy link
Author

treeowl commented Aug 22, 2021

(Personally, I like this version better than that one, but it's not my package.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant