[GHC] #14052: Significant GHCi speed regression with :module and `let` in GHC 8.2.1

[GHC]

#14052: Significant GHCi speed regression with :module and `let` in GHC 8.2.1
-------------------------------------+-------------------------------------
        Reporter:  RyanGlScott       |                Owner:  (none)
            Type:  bug               |               Status:  new
        Priority:  high              |            Milestone:  8.4.1
       Component:  GHCi              |              Version:  8.2.1-rc2
      Resolution:                    |             Keywords:
Operating System:  Unknown/Multiple  |         Architecture:
 Type of failure:  Runtime           |  Unknown/Multiple
  performance bug                    |            Test Case:
      Blocked By:                    |             Blocking:
 Related Tickets:                    |  Differential Rev(s):
       Wiki Page:                    |
-------------------------------------+-------------------------------------

Comment (by simonpj):

 Thanks for characterising this so well.  Here are some thoughts.

 * The `GlobalRdrEnv` maps an `OccName` to a `[GlobalRdrElt]`.  This was
 designed to handle the case where there are a handful of things `A.f`,
 `B.f`, etc. in scope, all with unqualified name `f`.  The list is not
 expected to be long.

 * But in this case, I believe that that the list is getting long; hence
 the problem.  Adding a DEBUG warning for this situation would be good.

 * Why is the list getting long?  Becuase we have
 {{{
   ghci> let x = True    -- Binds Ghci1.x
   ghci> let x = False   -- Binds Ghci2.x
   ghci> let x = True    -- Binds GHci3.x
   ...etc...
 }}}
   All those Ids are (rightly) kept in the `ic_tythings`.  But they are
 ''also'' all kept in the `ic_rn_gbl_env`.  (`Note [The interactive
 package]` in `HscTypes` and the following notes are of some help.)

 * I can't work out why we keep the shadowed `x`'s in the `ic_rn_gbl_env`.
 If we simply deleted them, all would be well.  After all, '''we do not
 expect the user to be able to refer to an old `x` with a qualified name
 `Ghci1.x`'''.

 * But consider this:
 {{{
   ghci> :load M    -- Brings `x` and `M.x` into scope
   ghci> x
   ghci> "Hello"
   ghci> M.x
   ghci> "hello"
   ghci> let x = True  -- Shadows `x`
   ghci> x             -- The locally bound `x`
                       -- NOT an ambiguous reference
   ghci> True
   ghci> M.x           -- M.x is still in scope!
   ghci> "Hello"
 }}}
   So when we add `x = True` we must not delete the `M.x` from the
 `GlobalRdrEnv`; rather we just want to make it "qualified only"; hence the
 `mk_fake-imp_spec` in `shadowName`.

 * Side note: this is similar to the Template Haskell case, described in
 `Note [GlobalRdrEnv shadowing]` in `RdrName`.
 {{{
       module M where
         f x = h [d| f = ....f....M.f... |]
 }}}
   In the declaration quote, the unqualified `f` should refer to the `f`
 bound by the quote, but the qualified `M.f` should refer to the top-level
 `f`.  So we don't want to delete that top-level binding from the
 `GlobalRdrEnv`; we just want to make it "qualified only"; hence the
 `mk_fake-imp_spec` in `shadowName`.

 My conclusion: we want a way to delete from the `GlobalRdrEnv` things
 bound by GHCi (like `Ghci1.x`), which we do not want to refer to in a
 qualified way.  How can we distinguish `Ghci1.x` from `M.x`?  Two
 possibilities

 * Easy but a bit hacky: we can look at the package in the `Name`.

 * In some ways nicer: use the `is_qual` field of the `ImpDeclSpec`.
 Currently it's a `Bool`:
   * True => `M.x` in in scope, but `x` is not
   * False => Both `M.x` and `x` are in scope

   We could provide a third possibility, to say that `x` is in scope but
 `M.x` is not.  We could use that for GHCi-bound Ids.

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