#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