
#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