Opened 2 years ago

Closed 2 years ago

#14224 closed bug (fixed)

zipWith does not inline

Reported by: dfeuer Owned by:
Priority: normal Milestone: 8.4.1
Component: Core Libraries Version: 8.2.1
Keywords: Cc:
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: Runtime performance bug Test Case:
Blocked By: Blocking:
Related Tickets: Differential Rev(s): Phab:D3986
Wiki Page:

Description

zipWith is currently defined

zipWith :: (a->b->c) -> [a]->[b]->[c]
zipWith _f []     _bs    = []
zipWith _f _as    []     = []
zipWith f  (a:as) (b:bs) = f a b : zipWith f as bs

For now (I gather that might change soon?), the fact that it's recursive means that it will never inline. But we really want it to inline when applied to a function, for the same reasons we want map to inline. If f is something like const, flip const, or a lazy constructor, it's wasteful to create a thunk to apply it. All three of those cases are common. So unless/until we start inlining recursive functions, we probably want to use

zipWith f = go where
  go [] _ = []
  go _ [] = []
  go (x:xs) (y:ys) = f x y : go xs ys

There will probably be some regressions, of course.

Change History (5)

comment:1 Changed 2 years ago by nomeata

Loopification in the current planned form does not apply here (zipWith is not tail-recursive), so if that is what you referring to: It won’t help here.

The proposed form looks reasonable.

comment:2 Changed 2 years ago by bgamari

I agree, separating out the recursive bit as proposed sounds quite reasonable.

Would you care to offer a patch? It would be a good idea to mention this is the changelog of base while you are at it.

comment:3 Changed 2 years ago by sighingnow

Differential Rev(s): Phab:D3986

comment:4 Changed 2 years ago by Ben Gamari <ben@…>

In 11d9615/ghc:

Make zipWith and zipWith3 inlinable.

Reviewers: austin, hvr, bgamari, dfeuer

Reviewed By: dfeuer

Subscribers: rwbarton, thomie

GHC Trac Issues: #14224

Differential Revision: https://phabricator.haskell.org/D3986

comment:5 Changed 2 years ago by bgamari

Resolution: fixed
Status: newclosed

Thanks sighingnow!

Note: See TracTickets for help on using tickets.