Skip to content

Commit e2b335d

Browse files
committed
Fix a space leak in list traversal
Previously, we used `zip` to define `itraverse` for lists. This led to two problems: 1. Because the zip fused with the index generator, it could *not* fuse with the argument. 2. I ran into situations where the zip *didn't* fuse with the index generator, so my code ended up actually building *and saving* `[0..]` as a CAF. That's a nasty space leak, as well as slow. Writing `itraverse` for lists using `foldr` directly seems to clear up these issues. Unboxing the counter manually should prevent `Int` boxes from being allocated when the passed function doesn't need them.
1 parent db6cfcb commit e2b335d

File tree

1 file changed

+25
-3
lines changed

1 file changed

+25
-3
lines changed

indexed-traversable/src/WithIndex.hs

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
{-# LANGUAGE GADTs #-}
77
{-# LANGUAGE TypeOperators #-}
88
{-# LANGUAGE UndecidableInstances #-}
9+
{-# LANGUAGE MagicHash #-}
10+
{-# LANGUAGE ScopedTypeVariables #-}
911

1012
#if __GLASGOW_HASKELL__ >= 702
1113
{-# LANGUAGE Trustworthy #-}
@@ -19,7 +21,7 @@ module WithIndex where
1921

2022
import Prelude
2123
(Either (..), Functor (..), Int, Maybe (..), Monad (..), Num (..), error,
22-
flip, id, seq, snd, ($!), ($), (.), zip)
24+
flip, id, seq, snd, ($!), ($), (.))
2325

2426
import Control.Applicative
2527
(Applicative (..), Const (..), ZipList (..), (<$>), liftA2)
@@ -45,6 +47,7 @@ import Data.Sequence (Seq)
4547
import Data.Traversable (Traversable (..))
4648
import Data.Tree (Tree (..))
4749
import Data.Void (Void)
50+
import GHC.Exts (Int (..), Int#, (+#))
4851

4952
#if __GLASGOW_HASKELL__ >= 702
5053
import GHC.Generics
@@ -263,9 +266,28 @@ instance FoldableWithIndex Int [] where
263266
go !n (x:xs) = f n x (go (n + 1) xs)
264267
{-# INLINE ifoldr #-}
265268
instance TraversableWithIndex Int [] where
266-
itraverse f = traverse (uncurry' f) . zip [0..]
269+
itraverse f = itraverseListStarting 0 f
267270
{-# INLINE itraverse #-}
268271

272+
-- | Traverse a list with the given starting index. We used to define
273+
-- traversals for @[]@ and 'NonEmpty' using 'Prelude.zip'. Unfortunately, this
274+
-- could sometimes fail to fuse (at least for the @[]@ case), leading to
275+
-- @[0..]@ being allocated as a CAF and walked on each traversal, which is both
276+
-- a space leak and slow. Using a manual counter puts a stop to that, and using
277+
-- 'foldr' gives us a chance of fusing with the argument. I didn't see
278+
-- similarly disastrous behavior with 'NonEmpty', but defining its traversal
279+
-- this way produces a rather more readable unfolding that I'm more confident
280+
-- won't go wrong somehow. Why do we unbox the counter by hand? GHC /can/ do
281+
-- that itself, but for some reason it only happens with @-O2@, and we use the
282+
-- standard @-O1@.
283+
itraverseListStarting :: forall f a b. Applicative f => Int -> (Int -> a -> f b) -> [a] -> f [b]
284+
itraverseListStarting (I# i0) f = \xs -> foldr go stop xs i0
285+
where
286+
go x r !i = liftA2 (:) (f (I# i) x) (r (i +# 1#))
287+
stop :: Int# -> f [b]
288+
stop !_i = pure []
289+
{-# INLINE itraverseListStarting #-}
290+
269291
-- TODO: we could experiment with streaming framework
270292
-- imapListFB f xs = build (\c n -> ifoldr (\i a -> c (f i a)) n xs)
271293

@@ -292,7 +314,7 @@ instance FoldableWithIndex Int NonEmpty where
292314
{-# INLINE ifoldMap #-}
293315
instance TraversableWithIndex Int NonEmpty where
294316
itraverse f ~(a :| as) =
295-
liftA2 (:|) (f 0 a) (traverse (uncurry' f) (zip [1..] as))
317+
liftA2 (:|) (f 0 a) (itraverseListStarting 1 f as)
296318
{-# INLINE itraverse #-}
297319

298320
-------------------------------------------------------------------------------

0 commit comments

Comments
 (0)