[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:  osa1
            Type:  bug               |               Status:  new
        Priority:  high              |            Milestone:  8.4.2
       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:  #11547            |  Differential Rev(s):
       Wiki Page:                    |
-------------------------------------+-------------------------------------
Changes (by osa1):

 * owner:  (none) => osa1
 * related:   => #11547


Comment:

 I think regardless of the performance problems, #11547 (Phab:D2447) should
 just be reverted. I think there were some misunderstanding in the ticket,
 and some questions are left unanswered in Phab:D2447, and I think there's
 really no utility of this patch. While I agree that having ~1000 shadowed
 names take ~40s to load is a problem, I also think that keeping shadowed
 variables is unnecessary.

 Here are some facts:

 - GHCi prompt works like `do` block, as noted by Ben in Phab:D2447, in
 [https://stackoverflow.com/questions/14052093/ghci-let-what-does-it-do
 this SO thread], in the
 [https://downloads.haskell.org/~ghc/latest/docs/html/users_guide/ghci.html
 #using-do-notation-at-the-prompt user manual] etc. so it's only expected
 to have the same shadowing behavior in the GHCi prompt.

 - Simon says in comment:1 in #11547 that we should be consistent in
 shadowing. I think we were already consistent previously. Values are
 shadowed, types are also shadowed, but shadowed types are still accessible
 in the promopt. His example:

 {{{
 > data A = A
 > let f A = Int
 > data A = XX
 > :type f
 }}}

 already worked on GHC 8.0.1:

 {{{
 $ ghci
 GHCi, version 8.0.1: http://www.haskell.org/ghc/  :? for help
 Loaded GHCi configuration from /home/omer/rcbackup/.ghci
 λ:1> data A = A
 λ:2> let f A = 123
 λ:3> data A = XX
 λ:4> :t f
 f :: Num t => Ghci1.A -> t
 λ:5> :info Ghci1.A
 data Ghci1.A = A        -- Defined at <interactive>:1:1
 }}}

 The question

 > But what is the user-facing specification? We need user-manual stuff
 explaining what all this Ghci4.foo stuff means. How would you know whether
 to say :t Ghci2.foo or :t Ghci3.foo? Can you list all the foo functions?
 Etc.

 is left unanswered. Ben also asks about the specification in Phab:D2447,
 and that also goes unanswered, but somehow the patch gets merged later on.

 We should at least have a motivating example, otherwise #11547 can also be
 fixed with a better error message and that'd cost us nothing in terms of
 performance and implementation simplicity and gives us the same benefits.

 After this ideas in comment:10 can still be implemented as an improvement
 (I'm still digesting that comment).

 Simon, in this sentence:

 > 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.

 Why do you think we don't expect user to be able to refer shadowed x with
 qualified name? That was the motivation for #11547 and Phab:D2447.

 Secondly, the example in comment:10 worked fine with GHC 8.0.1:

 {{{
 $ ghci M.hs
 GHCi, version 8.0.1: http://www.haskell.org/ghc/  :? for help
 Loaded GHCi configuration from /home/omer/rcbackup/.ghci
 [1 of 1] Compiling M                ( M.hs, interpreted )
 Ok, modules loaded: M.
 λ:1> import M
 λ:2> M.x
 0
 λ:3> x
 0
 λ:4> let x = 1
 λ:5> x
 1
 λ:6> M.x
 0
 }}}

 so there were really no problems that Phab:D2447 solved. I'll again claim
 that there's no utility of that patch and it should be reverted.

 I'll add a perf test for this ticket, and then try to digest comment:10.

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