Use foldr in place of explicit recursion and toList

[josephcsible]

Checklist:

HLint

• I've changed the exposed interface (add new reexports, remove reexports, rename reexported things, etc.).
  • I've updated hlint.dhall accordingly to my changes (add new rules for the new imports, remove old ones, when they are outdated, etc.).
  • I've generated the new .hlint.yaml file (see this instructions).

General

• I've updated the CHANGELOG with the short description of my latest changes.
• All new and existing tests pass.
• I keep the code style used in the files I've changed (see style-guide for more details).
• I've used the stylish-haskell file.
• My change requires the documentation updates.
  • I've updated the documentation accordingly.
• I've added the [ci skip] text to the docs-only related commit's name.

[chshersh]

@josephcsible Thanks for your work! However, foldr wasn't used in those functions on purpose. There exist known case where using toList increases performance and reduces the chance of having a space leak. See the following blog post for reference:

• https://www.snoyman.com/blog/2017/01/foldable-mapm-maybe-and-recursive-functions#comment-3093660760

I would love to be sure that this change is safe and doesn't introduce unexpected performance penalties before merging the PR.

[chshersh]

Hi @josephcsible! Let me know if you're interested in implementing benchmarks (in a separate repository) to see whether your PR changes the performance of those functions 🙂

[josephcsible]

Yes, I plan to do that. I would've by now, but I've been distracted by other things.

[chshersh]

@josephcsible No worries! Take your time 🙂

[vrom911]

Hey @josephcsible , sorry for bothering, do you have plans for this PR?

[josephcsible]

Thanks for the reminder; this slipped off my radar. A quick test seems to indicate that the new foldr approach actually has significantly better performance, in terms of both time and maximum memory alocation, than the old toList approach. My test program:

{-# LANGUAGE RankNTypes #-}

import Control.Monad (when)
import Data.Foldable (toList)
import System.Environment (getArgs)

allM_toList :: (Foldable f, Monad m) => (a -> m Bool) -> f a -> m Bool
allM_toList p = go . toList
  where
    go []     = pure True
    go (x:xs) = do
        q <- p x
        if q then go xs else pure False

allM_foldr :: (Foldable f, Monad m) => (a -> m Bool) -> f a -> m Bool
allM_foldr p = foldr go (pure True)
  where
    go x acc = do
        q <- p x
        if q then acc else pure False

testAllM :: (forall f m a. (Foldable f, Monad m) => (a -> m Bool) -> f a -> m Bool) -> IO ()
testAllM allM = do
  print $ allM Just (replicate 50000000 True)
  print $ allM Just (replicate 50000000 True ++ [False])
  print $ allM id (replicate 50000000 (Just True) ++ [Nothing])

main :: IO ()
main = do
  args <- getArgs
  when ("toList" `elem` args) $ do
    putStrLn "Using the toList variant"
    testAllM allM_toList
  when ("foldr" `elem` args) $ do
    putStrLn "Using the foldr variant"
    testAllM allM_foldr

And the results:

$ ghc -O Main.hs
[1 of 1] Compiling Main             ( Main.hs, Main.o )
Linking Main ...
$ GHCRTS=-s ./Main foldr
Using the foldr variant
Just True
Just False
Nothing
  22,000,095,408 bytes allocated in the heap
       3,468,296 bytes copied during GC
          60,960 bytes maximum residency (4 sample(s))
          29,152 bytes maximum slop
               0 MB total memory in use (0 MB lost due to fragmentation)

                                     Tot time (elapsed)  Avg pause  Max pause
  Gen  0     21083 colls,     0 par    0.038s   0.036s     0.0000s    0.0006s
  Gen  1         4 colls,     0 par    0.000s   0.000s     0.0001s    0.0001s

  INIT    time    0.000s  (  0.000s elapsed)
  MUT     time    4.050s  (  4.052s elapsed)
  GC      time    0.038s  (  0.036s elapsed)
  EXIT    time    0.000s  (  0.000s elapsed)
  Total   time    4.088s  (  4.089s elapsed)

  %GC     time       0.0%  (0.0% elapsed)

  Alloc rate    5,432,553,552 bytes per MUT second

  Productivity  99.1% of total user, 99.1% of total elapsed

$ GHCRTS=-s ./Main toList
Using the toList variant
Just True
Just False
Nothing
  22,000,095,424 bytes allocated in the heap
   8,811,405,376 bytes copied during GC
   2,567,085,096 bytes maximum residency (14 sample(s))
      40,764,376 bytes maximum slop
            2448 MB total memory in use (0 MB lost due to fragmentation)

                                     Tot time (elapsed)  Avg pause  Max pause
  Gen  0     21100 colls,     0 par    1.716s   1.716s     0.0001s    0.0006s
  Gen  1        14 colls,     0 par    3.547s   3.547s     0.2533s    1.6081s

  INIT    time    0.000s  (  0.000s elapsed)
  MUT     time    3.916s  (  3.916s elapsed)
  GC      time    5.262s  (  5.263s elapsed)
  EXIT    time    0.000s  (  0.000s elapsed)
  Total   time    9.179s  (  9.179s elapsed)

  %GC     time       0.0%  (0.0% elapsed)

  Alloc rate    5,617,801,221 bytes per MUT second

  Productivity  42.7% of total user, 42.7% of total elapsed

$

[chshersh]

@josephcsible Wow, great news! Looks like it's indeed much better than the previous implementation 👍 In that case, it would be indeed nice to have this feature merged.

[vrom911]

Amazing result! Thanks for your hard work, @josephcsible 👍