Skip to content

Foldable traversable#46

Merged
treeowl merged 3 commits intolspitzner:masterfrom
treeowl:foldable-traversable
Dec 7, 2021
Merged

Foldable traversable#46
treeowl merged 3 commits intolspitzner:masterfrom
treeowl:foldable-traversable

Conversation

@treeowl
Copy link
Copy Markdown
Collaborator

@treeowl treeowl commented Dec 7, 2021

Improve folding and traversing. Add mapMWithKey (for traversing in strict monads). Clean up cruft.

* Define `null` and `length`.
* Use `liftA2` in case that's better for the underlying `Applicative`.
@treeowl treeowl requested a review from konsumlamm December 7, 2021 04:35
@treeowl treeowl force-pushed the foldable-traversable branch from ec24da5 to 980e381 Compare December 7, 2021 06:31
@treeowl treeowl requested a review from lspitzner December 7, 2021 07:09
Copy link
Copy Markdown
Collaborator

@konsumlamm konsumlamm left a comment

Choose a reason for hiding this comment

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

I'm not sure I'm convinced about the need for mapMWithKey. I don't know of any similar function in other related packages. Do you have benchmarks that show a significant speedup?

Comment thread CHANGELOG.md Outdated
Comment thread pqueue.cabal
Comment thread src/Data/PQueue/Internals.hs
Comment thread src/Data/PQueue/Prio/Max.hs Outdated
Comment thread src/Data/PQueue/Prio/Max.hs Outdated
Comment thread src/Data/PQueue/Prio/Max.hs
Comment thread src/Data/PQueue/Prio/Max/Internals.hs
@treeowl
Copy link
Copy Markdown
Collaborator Author

treeowl commented Dec 7, 2021

I'm not sure I'm convinced about the need for mapMWithKey. I don't know of any similar function in other related packages. Do you have benchmarks that show a significant speedup?

I can write one. This is a general problem; deferring data structure construction until the very end (building up chains of closures) is awful.

@treeowl
Copy link
Copy Markdown
Collaborator Author

treeowl commented Dec 7, 2021

@konsumlamm, here's a test program:

{-# language BangPatterns #-}
module Main where
import Data.PQueue.Prio.Min (MinPQueue)
import qualified Data.PQueue.Prio.Min as Q
import Control.Monad.Trans.State.Strict

glump :: MinPQueue Int Int
glump = Q.fromList [(a, a) | a <- [1..2*10^6]]

hoom :: MinPQueue Int Int -> MinPQueue Int Int
hoom = flip evalState 0 . Q.mapMWithKey
  (\x y -> do
     !old <- get
     put (old + 1)
     pure (x + y + old))

main = print $ take 3 $ Q.toAscList (hoom glump)

On my system, this runs in (give or take)

real    0m1.688s
user    0m1.519s
sys     0m0.168s

If I use traverseWithKey instead, it runs in (give or take)

real    0m2.620s
user    0m2.363s
sys     0m0.256s

That seems to me like enough to care about.

@treeowl treeowl force-pushed the foldable-traversable branch from 980e381 to 9f2613e Compare December 7, 2021 17:34
* Add `mapMWithKey`, a version of `traverseWithKey` optimized for strict
  monads.

* Clean out some cruft.
@treeowl treeowl force-pushed the foldable-traversable branch from 9f2613e to 975e89e Compare December 7, 2021 17:45
@treeowl treeowl merged commit b815489 into lspitzner:master Dec 7, 2021
Copy link
Copy Markdown
Collaborator

@konsumlamm konsumlamm left a comment

Choose a reason for hiding this comment

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

Next time, please wait until the PR has been approved/re-reviewed before you merge, to make sure that the comments really have been addressed.

Comment thread src/Data/PQueue/Prio/Max.hs
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.

2 participants