Re: [GHC] #10528: compile time performance regression with OverloadedStrings and Text

27 Jul 2015

      #10528: compile time performance regression with OverloadedStrings and Text
-------------------------------------+-------------------------------------
        Reporter:  jakewheat         |                   Owner:  bgamari
            Type:  bug               |                  Status:  new
        Priority:  high              |               Milestone:  7.10.3
       Component:  Compiler          |                 Version:  7.10.2-rc2
      Resolution:                    |                Keywords:
Operating System:  Linux             |            Architecture:
 Type of failure:  Compile-time      |  Unknown/Multiple
  performance bug                    |               Test Case:
      Blocked By:                    |                Blocking:
 Related Tickets:                    |  Differential Revisions:
-------------------------------------+-------------------------------------

Comment (by simonpj):

 Aha!  RULE `TEXT literal` is terribly fragile:

  * In `GHC.Base` we have
 {{{
 {-# RULES
 "unpack"       [~1] forall a   . unpackCString# a             = build
 (unpackFoldrCString# a)
 "unpack-list"  [1]  forall a   . unpackFoldrCString# a (:) [] =
 unpackCString# a
 }}}
  * `build` inlines in phase 1

 So, transformation goes like this:
 {{{
 Start with
    fromString (unpackCString# "blah"#)

 ---> (simplify: gentle phase)  fire rule "unpack"

    fromString (build (unpackFoldrCString# "blah"#))

 ---> (simplify: phase2)  inline fromString

    Data.Text.pack (build (unpackFoldrCString# "blah"#))

 ---> (simplify: phase1)  inline pack, build, fire rule "unpack-list"

    unstream (map safe (streamList (unpackCString# "blah"#)))
 }}}
 Now you'd think that rule "TEXT literal" would now fire.  But its LHS too
 has been rewritten
 by RULE "unpack" to
 {{{
 "TEXT literal" [ALWAYS] forall a :: Addr#
   unstream (map
               safe
               (streamList
                  @ Char
                  (build @ Char (\ @ b -> unpackFoldrCString# @ b a))))
   = unpackCString# a
 }}}
 You can see this by doing `ghc --show-iface Data/Text/Show.hi`,
 incidentally.

 Why did that happen? Arguably it's a bug: we should not rewrite the LHS of
 a rule with other rules, any more than we should inline functions on the
 LHS of a rule.

 But it betrays a potential flaw in the rule setup. Consider a source-
 program expression `(unnstream (map safe (streamList (unpackCString#
 "foo"))))`.  Since there is a rule for `unpackCString#`, there is no
 guarantee that rule `TEXT literal` will fire before rule `unpack`.  In
 this case we know that the `unpackCString#` call will eventually be
 rewritten back into `unpackCString#`.  But it would be better to express
 that directly by saying
 {{{
 {-# RULES [2] "TEXT literal" forall a.
     unstream (S.map safe (S.streamList (GHC.unpackCString# a)))
       = unpackCString# a #-}
 }}}
 Now the two rules do not compete, and all will be good.

 So there are several things to say here

  * GHC probably should do no inlining and no rule rewriting on LHS of
 rules.  I'll fix that.

  * GHC already warns when an inlining competes with a rule; but it should
 also warn if a rule competes with a rule.  I'll fix that too.

  * To eliminate the warning, rule "TEXT literal" should be fixed as above.

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