[commit: ghc] master: Fix a huge space leak in the mighty Simplifier (45d9a15)

Fri May 22 14:14:45 UTC 2015

Repository : ssh://git@git.haskell.org/ghc

On branch  : master
Link       : http://ghc.haskell.org/trac/ghc/changeset/45d9a15c4b85a2ed89579106bdafd84accf2cb39/ghc

>---------------------------------------------------------------

commit 45d9a15c4b85a2ed89579106bdafd84accf2cb39
Author: Simon Peyton Jones <simonpj at microsoft.com>
Date:   Fri May 22 15:04:47 2015 +0100

    Fix a huge space leak in the mighty Simplifier

    This long-standing, terrible, adn somewhat subtle bug was exposed
    by Trac #10370, thanks to Reid Barton's brilliant test case (comment:3).

    The effect is large on the Trac #10370 test.
    Here is what the profile report says:

    Before:
     total time  =       24.35 secs   (24353 ticks @ 1000 us, 1 processor)
     total alloc = 11,864,360,816 bytes  (excludes profiling overheads)

    After:
     total time  =       21.16 secs   (21160 ticks @ 1000 us, 1 processor)
     total alloc = 7,947,141,136 bytes  (excludes profiling overheads)

    The /combined/ effect of the tidyOccName fix, plus this one, is dramtic
    for Trac #10370.  Here is what +RTS -s says:

    Before:
      15,490,210,952 bytes allocated in the heap
       1,783,919,456 bytes maximum residency (20 sample(s))

      MUT     time   30.117s  ( 31.383s elapsed)
      GC      time   90.103s  ( 90.107s elapsed)
      Total   time  120.843s  (122.065s elapsed)

    After:
       7,928,671,936 bytes allocated in the heap
          52,914,832 bytes maximum residency (25 sample(s))

      MUT     time   13.912s  ( 15.110s elapsed)
      GC      time    6.809s  (  6.808s elapsed)
      Total   time   20.789s  ( 21.954s elapsed)

    - Heap allocation halved
    - Residency cut by a factor of more than 30.
    - ELapsed time cut by a factor of 6

    Not bad!

    The details
    ~~~~~~~~~~~
    The culprit was SimplEnv.mkCoreSubst, which used mapVarEnv to do some
    impedence-matching from the substitituion used by the simplifier to
    the one used by CoreSubst.  But the impedence-mactching was recursive!

      mk_subst tv_env cv_env id_env
        = CoreSubst.mkSubst in_scope tv_env cv_env (mapVarEnv fiddle id_env)

      fiddle (DoneEx e)          = e
      fiddle (DoneId v)          = Var v
      fiddle (ContEx tv cv id e) = CoreSubst.substExpr (mk_subst tv cv id) e

    Inside fiddle, in the ContEx case, we may do another whole level of
    fiddle.  And so on.  Moreover, UniqFM (which is built on Data.IntMap) is
    strict, so the fiddling is done eagerly.  I didn't wok through all the
    details but the result is a gargatuan blow-up of entirely unnecessary work.

    Laziness would make this go away, I think, but I don't want to mess
    with IntMap.  And in any case, the impedence matching is a royal pain.

    In the end I simply ceased trying to use CoreSubst.substExpr in the
    simplifier, and instead just use simplExpr.  That does mean bit of
    duplication; e.g.  new code for simplRules.  But it's not a big deal
    and it's far more direct and easy to reason about.

    A bit of knock-on refactoring:

     * Data type ArgSummary moves to CoreUnfold.

     * interestingArg moves from CoreUnfold to SimplUtils, and gets a
       SimplEnv argument which can be used when we encounter a variable.

     * simplLamBndrs, addBndrRules move from SimplEnv to Simplify
       (because they now calls simplUnfolding, simplRules resp)

     * SimplUtils.substExpr, substUnfolding, mkCoreSubst die completely

     * In Simplify some several functions that were previously pure
       substitution-based functions are now monadic:
         - addBndrRules, simplRule
         - addCoerce, add_coerce in simplCast

     * In case 2c of Simplify.rebuildCase, there was a pretty disgusting
       expression-substitution taking place for 'rhs'; and we really don't
       want to make that monadic becuase 'rhs' can be big.
       Solution: reduce the arity of the rules for seq.
       See Note [User-defined RULES for seq] in MkId.

>---------------------------------------------------------------

45d9a15c4b85a2ed89579106bdafd84accf2cb39
 compiler/coreSyn/CoreUnfold.hs                     |  88 +-----
 compiler/simplCore/SimplCore.hs                    |  25 +-
 compiler/simplCore/SimplEnv.hs                     |  94 +-----
 compiler/simplCore/SimplUtils.hs                   | 217 +++++++++-----
 compiler/simplCore/Simplify.hs                     | 323 +++++++++++++--------
 testsuite/tests/perf/compiler/all.T                |   4 +-
 .../tests/simplCore/should_compile/T7785.stderr    |   2 +
 .../tests/simplCore/should_compile/rule2.stderr    |   3 +-
 testsuite/tests/simplCore/should_run/SeqRule.hs    |   2 +-
 9 files changed, 397 insertions(+), 361 deletions(-)

Diff suppressed because of size. To see it, use:

    git diff-tree --root --patch-with-stat --no-color --find-copies-harder --ignore-space-at-eol --cc 45d9a15c4b85a2ed89579106bdafd84accf2cb39