Re: [GHC] #13344: Core string literal patch regresses compiler performance considerably

2 Mar 2017

      #13344: Core string literal patch regresses compiler performance considerably
-------------------------------------+-------------------------------------
        Reporter:  bgamari           |                Owner:  bgamari
            Type:  bug               |               Status:  new
        Priority:  high              |            Milestone:  8.2.1
       Component:  Compiler          |              Version:  8.1
      Resolution:                    |             Keywords:
Operating System:  Unknown/Multiple  |         Architecture:
 Type of failure:  Compile-time      |  Unknown/Multiple
  performance bug                    |            Test Case:
      Blocked By:                    |             Blocking:
 Related Tickets:                    |  Differential Rev(s):
       Wiki Page:                    |
-------------------------------------+-------------------------------------

Comment (by rwbarton):

 The increase in interface file size is definitely not the only factor.
 Another known problem is that CSE is not working properly for top-level
 unboxed string literals. I found that I could make it work by changing the
 `Lit l` case in `use_subst` below, in CSE:
 {{{#!hs
 addBinding :: CSEnv                      -- Includes InId->OutId cloning
            -> InId
            -> OutId -> OutExpr           -- Processed binding
            -> (CSEnv, OutId)             -- Final env, final bndr
 -- Extend the CSE env with a mapping [rhs -> out-id]
 -- unless we can instead just substitute [in-id -> rhs]
 addBinding env in_id out_id rhs'
   | noCSE in_id = (env,                              out_id)
   | use_subst   = (extendCSSubst env in_id rhs',     out_id)
   | otherwise   = (extendCSEnv env rhs' id_expr', zapped_id)
   where
     id_expr'  = varToCoreExpr out_id
     zapped_id = zapIdUsageInfo out_id
        -- Putting the Id into the cs_map makes it possible that
        -- it'll become shared more than it is now, which would
        -- invalidate (the usage part of) its demand info.
        --    This caused Trac #100218.
        -- Easiest thing is to zap the usage info; subsequently
        -- performing late demand-analysis will restore it.  Don't zap
        -- the strictness info; it's not necessary to do so, and losing
        -- it is bad for performance if you don't do late demand
        -- analysis

     -- Should we use SUBSTITUTE or EXTEND?
     -- See Note [CSE for bindings]
     use_subst = case rhs' of
                    Var {} -> True
                    Lit l  -> litIsTrivial l
                    _      -> False
 }}}
 `litIsTrivial` is false for an unboxed string literal (and for an
 `Integer` literal, and True for other literals).

 However I don't really understand what is going on here. Is making
 `use_subst` return True for unboxed string literals the right fix?

 I also don't think fixing this is going to make a large change to total
 build time, since gridaphobe says that Phab:D1259 also fixes CSE for top-
 level string literals, yet it improved overall build time by less than 1%
 on my test system.

--
Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13344#comment:17
GHC http://www.haskell.org/ghc/
The Glasgow Haskell Compiler