[GHC] #13227: Loss of full laziness in mapFB

[GHC]

#13227: Loss of full laziness in mapFB
-------------------------------------+-------------------------------------
        Reporter:  simonpj           |                Owner:  nomeata
            Type:  bug               |               Status:  closed
        Priority:  normal            |            Milestone:
       Component:  Compiler          |              Version:  8.0.1
      Resolution:  fixed             |             Keywords:
Operating System:  Unknown/Multiple  |         Architecture:
                                     |  Unknown/Multiple
 Type of failure:  None/Unknown      |            Test Case:
      Blocked By:                    |             Blocking:
 Related Tickets:                    |  Differential Rev(s):  Phab:D3067
       Wiki Page:                    |
-------------------------------------+-------------------------------------

Comment (by simonpj):

 I fixed a small bug in the new one-shot stuff, in this patch
 {{{
     commit fc9d152b058f21ab03986ea722d0c94688b9969f
     Author: Simon Peyton Jones <    simonpj at microsoft.com    >
     Date:   Thu Feb 16 09:41:55 2017 +0000

         Comments and tiny refactor only
 }}}
 Here's the critical bit
 {{{
 --- a/compiler/simplCore/OccurAnal.hs
 +++ b/compiler/simplCore/OccurAnal.hs
 @@ -1867,17 +1867,17 @@ occAnalApp env (Var fun, args, ticks)
         -- This is the *whole point* of the isRhsEnv predicate
         -- See Note [Arguments of let-bound constructors]

 -    n_val_args = valArgCount args + length (takeWhile isOneShotInfo
 (occ_one_shots env))
 -        -- See Note [Sources of one-shot information], bullet point A'
 -
 +    n_val_args = valArgCount args
      n_args     = length args
      fun_uds    = mkOneOcc env fun (n_val_args > 0) n_args
      is_exp     = isExpandableApp fun n_val_args
          -- See Note [CONLIKE pragma] in BasicTypes
          -- The definition of is_exp should match that in
 Simplify.prepareRhs

 -    one_shots  = argsOneShots (idStrictness fun) n_val_args
 -        -- See Note [Sources of one-shot information]
 +    one_shots  = argsOneShots (idStrictness fun) guaranteed_val_args
 +    guaranteed_val_args = n_val_args + length (takeWhile isOneShotInfo
 +                                                         (occ_one_shots
 env))
 +        -- See Note [Sources of one-shot information], bullet point A']
 }}}
 Notice that `guaranteed_val_args` should be used only for the call to
 `argOneShots`, not in the calls to `isExpandableApp` or `mkOneOcc`.

 I thoght this was just cleanup.  For example, `is_exp` only matters if
 `isRhsEnv` is true; and in that case I think `occ_one_shots` is empty (see
 `rhsCtxt`); so I doubt the change to `is_exp` makes any difference at all.

 Nevertheless it does: we observed a 7% reduction in allocation for
 `haddock.base` and `haddock.Cabal` after this one patch.  Bonkers!  I have
 no idea why.  But I'm just recording it here.

--
Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/13227#comment:21>
GHC <http://www.haskell.org/ghc/>
The Glasgow Haskell Compiler