[GHC] #13426: compile-time memory-usage regression for DynFlags between GHC 8.0 and GHC 8.2
#13426: compile-time memory-usage regression for DynFlags between GHC 8.0 and GHC
8.2
-------------------------------------+-------------------------------------
Reporter: hvr | Owner: (none)
Type: bug | Status: new
Priority: high | Milestone: 8.2.1
Component: Compiler | Version: 8.1
Keywords: | Operating System: Unknown/Multiple
Architecture: | Type of failure: Compile-time
Unknown/Multiple | performance bug
Test Case: | Blocked By:
Blocking: | Related Tickets:
Differential Rev(s): | Wiki Page:
-------------------------------------+-------------------------------------
I notice that buildbots were having a hard-time building GHC 8.2
snapshots... so I investigated...
One thing that jumped at me was the memory usage while compiling DynFlags
which is the worst memory-offender with 4777MiB of resident size:
{{{
"inplace/bin/ghc-stage1" -hisuf hi -osuf o -hcsuf hc -static -H32m -O
-Wall -Iincludes -Iincludes/dist -Iincludes/dist-derivedconstants/header
-Iincludes/dist-ghcconstants/header -this-unit-id ghc-8.2.0.201
70313 -hide-all-packages -i -icompiler/backpack -icompiler/basicTypes
-icompiler/cmm -icompiler/codeGen -icompiler/coreSyn -icompiler/deSugar
-icompiler/ghci -icompiler/hsSyn -icompiler/iface -icompiler/llvmGen -
icompiler/main -icompiler/nativeGen -icompiler/parser -icompiler/prelude
-icompiler/profiling -icompiler/rename -icompiler/simplCore
-icompiler/simplStg -icompiler/specialise -icompiler/stgSyn
-icompiler/stranal
-icompiler/typecheck -icompiler/types -icompiler/utils
-icompiler/vectorise -icompiler/stage2/build -Icompiler/stage2/build
-icompiler/stage2/build/./autogen -Icompiler/stage2/build/./autogen
-Icompiler/. -Icompi
ler/parser -Icompiler/utils -Icompiler/../rts/dist/build -Icompiler/stage2
-optP-DGHCI -optP-include
-optPcompiler/stage2/build/./autogen/cabal_macros.h -package-id
base-4.10.0.0 -package-id deepseq-1.4.3.0 -pa
ckage-id directory-1.3.0.2 -package-id process-1.4.3.0 -package-id
bytestring-0.10.8.2 -package-id binary-0.8.4.1 -package-id time-1.8.0.1
-package-id containers-0.5.10.2 -package-id array-0.5.1.2 -package-id fil
epath-1.4.1.1 -package-id template-haskell-2.12.0.0 -package-id
hpc-0.6.0.3 -package-id transformers-0.5.2.0 -package-id ghc-
boot-8.2.0.20170313 -package-id ghc-boot-th-8.2.0.20170313 -package-id
ghci-8.2.0.20170
313 -package-id hoopl-3.10.2.2 -package-id unix-2.7.2.1 -package-id
terminfo-0.4.0.2 -Wall -fno-warn-name-shadowing -this-unit-id ghc
-XHaskell2010 -optc-DTHREADED_RTS -DGHCI_TABLES_NEXT_TO_CODE -DSTAGE=2
-Rghc-t
iming -O2 -no-user-package-db -rtsopts -Wnoncanonical-monad-
instances -odir compiler/stage2/build -hidir compiler/stage2/build
-stubdir compiler/stage2/build -dynamic-too -c compiler/main/DynFlags.hs
-o
compiler/stage2/build/DynFlags.o -dyno
compiler/stage2/build/DynFlags.dyn_o
}}}
{{{
<
#13426: compile-time memory-usage regression for DynFlags between GHC 8.0 and GHC 8.2 -------------------------------------+------------------------------------- Reporter: hvr | Owner: (none) Type: bug | Status: new Priority: high | Milestone: 8.2.1 Component: Compiler | Version: 8.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Compile-time | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Changes (by hvr): * Attachment "ghc82-stage0-verbose-output.log" added. Compiling GHC 8.2's stage1 DynFlags.hs w/ stage0 (GHC 8.0.2) -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13426 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13426: compile-time memory-usage regression for DynFlags between GHC 8.0 and GHC 8.2 -------------------------------------+------------------------------------- Reporter: hvr | Owner: (none) Type: bug | Status: new Priority: high | Milestone: 8.2.1 Component: Compiler | Version: 8.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Compile-time | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Changes (by hvr): * Attachment "ghc82-stage1-verbose-output.log" added. Compiling GHC 8.2's stage2 DynFlags.hs w/ stage1 (GHC 8.2) -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13426 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13426: compile-time memory-usage regression for DynFlags between GHC 8.0 and GHC 8.2 -------------------------------------+------------------------------------- Reporter: hvr | Owner: (none) Type: bug | Status: new Priority: high | Milestone: 8.2.1 Component: Compiler | Version: 8.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Compile-time | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by simonpj): That is bad! Let's find out why. Something that egregious MUST have low-handing fruit. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13426#comment:1 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13426: compile-time memory-usage regression for DynFlags between GHC 8.0 and GHC 8.2 -------------------------------------+------------------------------------- Reporter: hvr | Owner: (none) Type: bug | Status: new Priority: high | Milestone: 8.2.1 Component: Compiler | Version: 8.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Compile-time | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by rwbarton): I have similar numbers from after the join points commit (Feb 1) and they do not show this large regression. So my educated guess is that the new Typeable stuff is responsible. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13426#comment:2 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13426: compile-time memory-usage regression for DynFlags between GHC 8.0 and GHC
8.2
-------------------------------------+-------------------------------------
Reporter: hvr | Owner: (none)
Type: bug | Status: new
Priority: high | Milestone: 8.2.1
Component: Compiler | Version: 8.1
Resolution: | Keywords:
Operating System: Unknown/Multiple | Architecture:
Type of failure: Compile-time | Unknown/Multiple
performance bug | Test Case:
Blocked By: | Blocking:
Related Tickets: | Differential Rev(s):
Wiki Page: |
-------------------------------------+-------------------------------------
Comment (by hvr):
Ben suggested that reverting 2effe18ab51d66474724d38b20e49cc1b8738f60
could have an interesting effect... however it appears to have things
worse :-)
{{{
<
#13426: compile-time memory-usage regression for DynFlags between GHC 8.0 and GHC 8.2 -------------------------------------+------------------------------------- Reporter: hvr | Owner: (none) Type: bug | Status: new Priority: high | Milestone: 8.2.1 Component: Compiler | Version: 8.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Compile-time | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Changes (by hvr): * Attachment "ghc82-stage1-verbose-output.revert.log" added. Compiling GHC 8.2's stage2 DynFlags.hs w/ stage1 (GHC 8.2) & 2effe18ab reverted -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13426 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13426: compile-time memory-usage regression for DynFlags between GHC 8.0 and GHC 8.2 -------------------------------------+------------------------------------- Reporter: hvr | Owner: (none) Type: bug | Status: new Priority: high | Milestone: 8.2.1 Component: Compiler | Version: 8.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Compile-time | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Changes (by hvr): * Attachment "ghc82-stage1-verbose-output.O1.log" added. Compiling GHC 8.2's stage2 DynFlags.hs w/ stage1 (GHC 8.2) (at -O1 instead of -O2; 2effe is NOT reverted here) -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13426 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13426: compile-time memory-usage regression for DynFlags between GHC 8.0 and GHC 8.2 -------------------------------------+------------------------------------- Reporter: hvr | Owner: (none) Type: bug | Status: new Priority: high | Milestone: 8.2.1 Component: Compiler | Version: 8.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Compile-time | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Changes (by niteria): * cc: niteria (added) Comment: I've found myself unable to build GHC inside vagrant running on VirtualBox with 7GB of memory. It always gets killed by OOM killer when compiling DynFlags. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13426#comment:4 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13426: compile-time memory-usage regression for DynFlags between GHC 8.0 and GHC 8.2 -------------------------------------+------------------------------------- Reporter: hvr | Owner: (none) Type: bug | Status: new Priority: high | Milestone: 8.2.1 Component: Compiler | Version: 8.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Compile-time | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Changes (by bgamari): * Attachment "ghc-8.0.svg" added. Building DynFlags with GHC 8.0 -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13426 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13426: compile-time memory-usage regression for DynFlags between GHC 8.0 and GHC 8.2 -------------------------------------+------------------------------------- Reporter: hvr | Owner: (none) Type: bug | Status: new Priority: high | Milestone: 8.2.1 Component: Compiler | Version: 8.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Compile-time | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Changes (by bgamari): * Attachment "ghc-8.2.svg" added. Building DynFlags with GHC 8.2 -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13426 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13426: compile-time memory-usage regression for DynFlags between GHC 8.0 and GHC 8.2 -------------------------------------+------------------------------------- Reporter: hvr | Owner: (none) Type: bug | Status: new Priority: high | Milestone: 8.2.1 Component: Compiler | Version: 8.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Compile-time | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by bgamari): Indeed something catastrophic is happening here; this can be plainly seen in the `+RTS -h` profiles I attached above. These were produced by building two GHC trees, one bootstrapping with 8.2 and another with 8.0.2, taking the `DynFlags` command line from the stage1 build, and adding `-fforce-recomp -v +RTS -h >log 2>&1` to it. Clearly we have a significant amount of leakage. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13426#comment:5 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13426: compile-time memory-usage regression for DynFlags between GHC 8.0 and GHC 8.2 -------------------------------------+------------------------------------- Reporter: hvr | Owner: (none) Type: bug | Status: new Priority: high | Milestone: 8.2.1 Component: Compiler | Version: 8.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Compile-time | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Changes (by bgamari): * Attachment "ghc-8.0.log" added. Log output from GHC 8.0 -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13426 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13426: compile-time memory-usage regression for DynFlags between GHC 8.0 and GHC 8.2 -------------------------------------+------------------------------------- Reporter: hvr | Owner: (none) Type: bug | Status: new Priority: high | Milestone: 8.2.1 Component: Compiler | Version: 8.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Compile-time | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Changes (by bgamari): * Attachment "ghc-8.2.log" added. Log output from GHC 8.2 -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13426 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13426: compile-time memory-usage regression for DynFlags between GHC 8.0 and GHC 8.2 -------------------------------------+------------------------------------- Reporter: hvr | Owner: (none) Type: bug | Status: new Priority: high | Milestone: 8.2.1 Component: Compiler | Version: 8.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Compile-time | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by rwbarton): I bisected and found out that the offending commit was none other than 8d5cf8bf584fd4849917c29d82dcf46ee75dd035 "Join points". I didn't see this in my previous data because, ironically, `-dcore-lint` causes the large space usage to go away. So it seems likely to be a garden-variety thunk buildup in some field which is examined by core lint, but otherwise not examined until late in the compilation pipeline. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13426#comment:6 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13426: compile-time memory-usage regression for DynFlags between GHC 8.0 and GHC 8.2 -------------------------------------+------------------------------------- Reporter: hvr | Owner: (none) Type: bug | Status: new Priority: high | Milestone: 8.2.1 Component: Compiler | Version: 8.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Compile-time | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by simonpj): Great work. There is definitely a space leak here and it should not be to hard to find. Some ideas * In `CoreLint.lintSingleBinding` you'll find a series of tests. If you comment then out one by one, you'll find when the space leak comes back. That test must be forcing the offending bit of `IdInfo`. * Does the leak happen when you use `-dshow-passes`? That shows the size of eac intermediate, and `exprSize` is deliberately strict (see `CoreStats.bndrSize`). You could try making `CoreSeq.megaSeqidInfo` actually force the unfolding info (currently commented out) and see if that helps. (I'm not sure WHY it is commented out, incidentally.) -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13426#comment:7 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13426: compile-time memory-usage regression for DynFlags between GHC 8.0 and GHC 8.2 -------------------------------------+------------------------------------- Reporter: hvr | Owner: (none) Type: bug | Status: new Priority: high | Milestone: 8.2.1 Component: Compiler | Version: 8.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Compile-time | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by rwbarton): Replying to [comment:7 simonpj]:
Great work. There is definitely a space leak here and it should not be to hard to find. Some ideas
* In `CoreLint.lintSingleBinding` you'll find a series of tests. If you comment then out one by one, you'll find when the space leak comes back. That test must be forcing the offending bit of `IdInfo`.
I commented out `lintIdUnfolding` and then the leak came back with `-dcore-lint` enabled.
* Does the leak happen when you use `-dshow-passes`? That shows the size of eac intermediate, and `exprSize` is deliberately strict (see `CoreStats.bndrSize`).
I've been using `-v`, which I think shows the same information, and it doesn't eliminate the leak.
You could try making `CoreSeq.megaSeqidInfo` actually force the unfolding info (currently commented out) and see if that helps. (I'm not sure WHY it is commented out, incidentally.)
Instead I defined a new version of `seqUnfolding`, based on `lintIdUnfolding`, and added it to `megaSeqIdInfo`: {{{#!hs miniSeqUnfolding :: Unfolding -> () miniSeqUnfolding (CoreUnfolding { uf_tmpl = e, uf_src = src }) | isStableSource src = seqExpr e miniSeqUnfolding (DFunUnfolding { df_con = con, df_bndrs = bndrs , df_args = args }) = seqExprs args miniSeqUnfolding _ = () }}} This was enough to make the leak go away, even without using `-dcore- lint`. Both the `CoreUnfolding` and `DFunUnfolding` cases are needed to make the leak go away; or perhaps there are two leaks. (Without the `isStableSource` guard, the runtime exploded.) So there are thunks in the expressions inside unfoldings which, I believe, point to old versions of the Core program. I'm going to continue invesigating but I'm out of time for today. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13426#comment:8 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13426: compile-time memory-usage regression for DynFlags between GHC 8.0 and GHC 8.2 -------------------------------------+------------------------------------- Reporter: hvr | Owner: (none) Type: bug | Status: new Priority: high | Milestone: 8.2.1 Component: Compiler | Version: 8.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Compile-time | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by rwbarton): I now suspect this leak doesn't really have anything to do with unfoldings in particular. It just shows up that way as a result of the fact that the evaluation of `coreBindsSize` in the simplifier loop forces most parts of the Core program, but does not force unfoldings (as mentioned in comment:7). It seems that the simplifier repeatedly traverses these stable `Core` and `DFun` unfoldings but there is some part of the new unfolding that the next simplifier iteration does not look at that retains a reference to the old version of the program. I also suspect that the evaluation of `coreBindsSize` in the simplifier loop should be unnecessary if there weren't space leaks in the simplifier, and if we got rid of it we would see this space leak not just in unfoldings but everywhere, and possibly in more programs, which might make it easier to track down. I spent a lot of time using some RTS functions to dump heap representations and in particular to look for thunks and (using DWARF information) find where they were coming from. But it turned out not to be a good approach because of how the simplifier is structured. Basically, the output of each simplifier iteration contains a large number of thunks, of which most (but apparently not all) will be forced by the next simplifier iteration, and replaced by a large number of new thunks. I don't know how to find only the thunks that aren't getting forced by this process. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13426#comment:9 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13426: compile-time memory-usage regression for DynFlags between GHC 8.0 and GHC 8.2 -------------------------------------+------------------------------------- Reporter: hvr | Owner: (none) Type: bug | Status: new Priority: high | Milestone: 8.2.1 Component: Compiler | Version: 8.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Compile-time | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by simonpj): Very useful to know that the stable unfoldings alone are enough. I have an idea. Consider `Simplify` line 1005 {{{ simplExprF1 env (Type ty) cont = ASSERT( contIsRhsOrArg cont ) rebuild env (Type (substTy env ty)) cont }}} Yikes! That `(substTy env ty)` is a thunk that the simplifier may not force; and that'll hold onto `env` which is disater. Replace the RHS by {{{ = do { ty' <- simplType env ty ; retbuild env (Type ty') cont } }}} You'll see that `simplType` is careful to `seq` on substituted type; and `TyCoRep.substTy` has the property (I think, though it is sadly not documented) that `seq`ing on the result is enough to push the substitution right through the type (see the `$!` calls in `subst_ty`. It is very unsatisfactory that this kind of leak is SO hard to find. But finding it will help ALL programs! -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13426#comment:10 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13426: compile-time memory-usage regression for DynFlags between GHC 8.0 and GHC 8.2 -------------------------------------+------------------------------------- Reporter: hvr | Owner: (none) Type: bug | Status: new Priority: high | Milestone: 8.2.1 Component: Compiler | Version: 8.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Compile-time | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by rwbarton): Replying to [comment:10 simonpj]:
I have an idea. Consider `Simplify` line 1005 {{{ simplExprF1 env (Type ty) cont = ASSERT( contIsRhsOrArg cont ) rebuild env (Type (substTy env ty)) cont }}}
This wasn't enough unfortunately. Some more RTS/profiling hackery led me to... the next case of the same function: {{{ simplExprF1 env (App fun arg) cont = simplExprF env fun $ case arg of Type ty -> ApplyToTy { sc_arg_ty = substTy env ty , sc_hole_ty = substTy env (exprType fun) , sc_cont = cont } _ -> ApplyToVal { sc_arg = arg, sc_env = env , sc_dup = NoDup, sc_cont = cont } }}} Here `sc_arg_ty` and `sc_hole_ty` apparently need a similar treatment. And with that the space leak seems to be gone! I'll confirm with a clean rebuild. What I still don't understand is why this wasn't always a problem, since those lines have not changed in a long time. I guess there is some new code that manipulates `ApplyToTy`, so maybe the old version of that code did enough evaluation to avoid the space leak. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13426#comment:11 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13426: compile-time memory-usage regression for DynFlags between GHC 8.0 and GHC 8.2 -------------------------------------+------------------------------------- Reporter: hvr | Owner: rwbarton Type: bug | Status: new Priority: high | Milestone: 8.2.1 Component: Compiler | Version: 8.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Compile-time | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Changes (by rwbarton): * owner: (none) => rwbarton -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13426#comment:12 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13426: compile-time memory-usage regression for DynFlags between GHC 8.0 and GHC 8.2 -------------------------------------+------------------------------------- Reporter: hvr | Owner: rwbarton Type: bug | Status: new Priority: high | Milestone: 8.2.1 Component: Compiler | Version: 8.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Compile-time | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by simonpj): Well done! Yes, use `simplType`! Can you add a Note to `simplType` explaining why it is so important to use it rather than naked `substTy` calls; and pointing to this ticket? I suspect that it has been a problem for a long time; just that no one has noticed. It's very uncomfortable that this is so delicate; and even when recognised is so hard to find. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13426#comment:13 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13426: compile-time memory-usage regression for DynFlags between GHC 8.0 and GHC 8.2 -------------------------------------+------------------------------------- Reporter: hvr | Owner: rwbarton Type: bug | Status: new Priority: high | Milestone: 8.2.1 Component: Compiler | Version: 8.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Compile-time | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by simonpj): PS: you may then be able to get rid of the call to `coreBindsSize` in `SimplCore`. If so, that would be good: faster compilation! Worth trying... it might I suppose show up another leak currently patched by `coreBindsSize`. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13426#comment:14 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
PS: you may then be able to get rid of the call to `coreBindsSize` in `SimplCore`. If so, that would be good: faster compilation! Worth
#13426: compile-time memory-usage regression for DynFlags between GHC 8.0 and GHC 8.2 -------------------------------------+------------------------------------- Reporter: hvr | Owner: rwbarton Type: bug | Status: new Priority: high | Milestone: 8.2.1 Component: Compiler | Version: 8.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Compile-time | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by rwbarton): There was actually a third required change which I had guessed at earlier, namely changing {{{#!hs simplExprF1 env (Case scrut bndr _ alts) cont = simplExprF env scrut (Select { sc_dup = NoDup, sc_bndr = bndr }}} to {{{#!hs simplExprF1 env (Case scrut bndr ty alts) cont = seqType ty `seq` simplExprF env scrut (Select { sc_dup = NoDup, sc_bndr = bndr }}} Alternatively, one can insert a `seqType` into `reallyRebuildCase`: {{{#!diff @@ -2222,7 +2223,7 @@ reallyRebuildCase env scrut case_bndr alts cont ; dflags <- getDynFlags ; let alts_ty' = contResultType dup_cont - ; case_expr <- seqType alts_ty' `seq` mkCase dflags scrut' case_bndr' alts_ty' alts' + ; case_expr <- mkCase dflags scrut' case_bndr' alts_ty' alts' }}} I don't understand specifically why these changes make any difference, especially the first one, where we were throwing away the `ty` field anyways. It must share structure with some other part of the program, but I'm not sure what. Replying to [comment:14 simonpj]: trying... it might I suppose show up another leak currently patched by `coreBindsSize`. Yes, I'll try that next as I have a good setup for finding leaks now. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13426#comment:15 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13426: compile-time memory-usage regression for DynFlags between GHC 8.0 and GHC 8.2 -------------------------------------+------------------------------------- Reporter: hvr | Owner: rwbarton Type: bug | Status: new Priority: high | Milestone: 8.2.1 Component: Compiler | Version: 8.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Compile-time | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by simonpj): Wow, I have no idea how you found that! Well done.
Alternatively, one can insert a seqType into reallyRebuildCase:
That would be ''much'' better. That kills the leak when we build a `Case`, whereas the former alternative waits until the ''subsequent'' run of the simplifier before squashing it. Please add a `Note` to explain the `seq` (point to this ticket too). Does it need to be a `seqType` or will `seq` do? I expect that `seq` would be enough, since the `OutType` returned by `contResultType` is (I think) a result of calling `simplType`.
It must share structure with some other part of the program, but I'm not sure what.
My guess: `exprType` often returns that type. Still I'm surprised that it leads to a ''cumulative'' space leak (i.e. every growing). Does it? -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13426#comment:16 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
PS: you may then be able to get rid of the call to `coreBindsSize` in `SimplCore`. If so, that would be good: faster compilation! Worth
#13426: compile-time memory-usage regression for DynFlags between GHC 8.0 and GHC 8.2 -------------------------------------+------------------------------------- Reporter: hvr | Owner: rwbarton Type: bug | Status: new Priority: high | Milestone: 8.2.1 Component: Compiler | Version: 8.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Compile-time | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D3399, Wiki Page: | Phab:D3400 -------------------------------------+------------------------------------- Changes (by rwbarton): * differential: => Phab:D3399, Phab:D3400 Comment: Replying to [comment:14 simonpj]: trying... it might I suppose show up another leak currently patched by `coreBindsSize`. This line of investigation already paid some dividends. It turns out that that `coreBindsSize` is what takes care of forcing the demand and strictness info produced by the demand analysis pass. Otherwise, the demand analysis pass leaks basically a copy of the whole Core program. But ''late'' demand analysis runs after all simplifier passes, and there is no more `coreBindsSize` or `seqBinds` after that. Presumably code generation uses some of the information added by this pass, but it doesn't force all of it; and the result is that we hang on to two copies of the Core program during code generation. And code generation is the phase that uses the most memory anyways, so this makes a big difference in peak memory usage. As a crude measure I added a `seqBinds` to demand analysis (Phab:D3400) and it reduced peak memory usage on DynFlags by around 30% and even made the running time a few percent faster (I think because of reduced GC work). -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13426#comment:17 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
PS: you may then be able to get rid of the call to `coreBindsSize` in `SimplCore`. If so, that would be good: faster compilation! Worth
#13426: compile-time memory-usage regression for DynFlags between GHC 8.0 and GHC 8.2 -------------------------------------+------------------------------------- Reporter: hvr | Owner: rwbarton Type: bug | Status: new Priority: high | Milestone: 8.2.1 Component: Compiler | Version: 8.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Compile-time | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D3399, Wiki Page: | Phab:D3400 -------------------------------------+------------------------------------- Comment (by rwbarton): Replying to [comment:14 simonpj]: trying... it might I suppose show up another leak currently patched by `coreBindsSize`. The input core program size is used to bound the number of simplifier ticks so we can't remove the call to `coreBindsSize` entirely without affecting the behavior of the simplifier. I'm not sure whether we could make do with a more crude estimate of program size. However, `coreBindsSize` currently goes out of its way to force a lot of structure (like `seqBinds` does) that is not needed to compute the Core program size. So an easy intermediate option is to just remove all the extra `seq`s from `coreBindsSize`. This still leaves a lot less work than `coreBindsSize` does currently and forcing the main program body may be useful. I almost have this working but in order to fix another space leak I had to make another similar change of {{{#!hs simplExpr env expr = simplExprC env expr (mkBoringStop expr_out_ty) where expr_out_ty :: OutType expr_out_ty = substTy env (exprType expr) }}} to {{{#!hs simplExpr env expr = do { expr_out_ty <- simplType env (exprType expr) ; simplExprC env expr (mkBoringStop expr_out_ty) } }}} However, this also produces a lot of warnings like {{{ exprType TYPE: ModuleName }}} which seem to be from calling `simplExpr` on a `Type t`. Is it a bug that that happens in the first place? What is the best way to handle this? I get another smal improvement in space and time from this change so I'm hoping this is easy to finish off. Please advise! -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13426#comment:18 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13426: compile-time memory-usage regression for DynFlags between GHC 8.0 and GHC 8.2 -------------------------------------+------------------------------------- Reporter: hvr | Owner: rwbarton Type: bug | Status: new Priority: high | Milestone: 8.2.1 Component: Compiler | Version: 8.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Compile-time | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D3399, Wiki Page: | Phab:D3400 -------------------------------------+------------------------------------- Comment (by simonpj): About forcing the type before passing to `mkBoringStop`. Often the type passed into `mkBoringStop` is discarded unused, so I'd prefer not to force it when building a `SimplCont`. I'd prefer to force it when getting it out of the `SimplCont`. Notably, `contResultType` does that. So, possible alternative plan: when extracting the `contResultType` can we just force it then? E.g. {{{ ; let alts_ty' = contResultType dup_cont ; case_expr <- mkCase dflags scrut' case_bndr' alts_ty' alts' }}} Aha -- you have already added a `seq` there! Here's another (line 1622 of `Simplify`) {{{ missingAlt env case_bndr _ cont = WARN( True, text "missingAlt" <+> ppr case_bndr ) return (env, mkImpossibleExpr (contResultType cont)) }}} Here want to force that type. Ditto here (line 1656) {{{ | not (contIsTrivial cont) -- Only do this if there is a non-trivial = return (env, castBottomExpr res cont_ty) -- continuation to discard, else we do it where -- again and again! res = argInfoExpr fun rev_args cont_ty = contResultType cont }}} But not here (line 1591) {{{ trim_cont 0 cont = mkBoringStop (contResultType cont) }} because we are just building another `SimplCont`. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13426#comment:19 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13426: compile-time memory-usage regression for DynFlags between GHC 8.0 and GHC
8.2
-------------------------------------+-------------------------------------
Reporter: hvr | Owner: rwbarton
Type: bug | Status: new
Priority: high | Milestone: 8.2.1
Component: Compiler | Version: 8.1
Resolution: | Keywords:
Operating System: Unknown/Multiple | Architecture:
Type of failure: Compile-time | Unknown/Multiple
performance bug | Test Case:
Blocked By: | Blocking:
Related Tickets: | Differential Rev(s): Phab:D3399,
Wiki Page: | Phab:D3400
-------------------------------------+-------------------------------------
Comment (by Simon Peyton Jones
#13426: compile-time memory-usage regression for DynFlags between GHC 8.0 and GHC 8.2 -------------------------------------+------------------------------------- Reporter: hvr | Owner: rwbarton Type: bug | Status: new Priority: high | Milestone: 8.2.1 Component: Compiler | Version: 8.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Compile-time | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D3399, Wiki Page: | Phab:D3400 -------------------------------------+------------------------------------- Comment (by simonpj):
Please advise!
I've pushed a fix (to HEAD) that should resolve this problem. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13426#comment:21 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13426: compile-time memory-usage regression for DynFlags between GHC 8.0 and GHC
8.2
-------------------------------------+-------------------------------------
Reporter: hvr | Owner: rwbarton
Type: bug | Status: new
Priority: high | Milestone: 8.2.1
Component: Compiler | Version: 8.1
Resolution: | Keywords:
Operating System: Unknown/Multiple | Architecture:
Type of failure: Compile-time | Unknown/Multiple
performance bug | Test Case:
Blocked By: | Blocking:
Related Tickets: | Differential Rev(s): Phab:D3399,
Wiki Page: | Phab:D3400
-------------------------------------+-------------------------------------
Comment (by Ben Gamari
#13426: compile-time memory-usage regression for DynFlags between GHC 8.0 and GHC 8.2 -------------------------------------+------------------------------------- Reporter: hvr | Owner: rwbarton Type: bug | Status: closed Priority: high | Milestone: 8.2.1 Component: Compiler | Version: 8.1 Resolution: fixed | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Compile-time | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D3399, Wiki Page: | Phab:D3400 -------------------------------------+------------------------------------- Changes (by bgamari): * status: new => closed * resolution: => fixed Comment: Comment;20 and comment:22 merged to `ghc-8.2` as 4d724299a3aedfaec99b7d14187736fe5886de1a and 0d20cc11c95de33b8a125555c16d2b0860030227. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13426#comment:23 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13426: compile-time memory-usage regression for DynFlags between GHC 8.0 and GHC 8.2 -------------------------------------+------------------------------------- Reporter: hvr | Owner: (none) Type: bug | Status: new Priority: high | Milestone: 8.2.1 Component: Compiler | Version: 8.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Compile-time | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D3399, Wiki Page: | Phab:D3400 -------------------------------------+------------------------------------- Changes (by simonpj): * status: closed => new * owner: rwbarton => (none) * resolution: fixed => Comment: Ben, I'm puzzled here. This commit seems to implement comment:15 (in `rebuildCase`) but not comment:11 and its antecedents. How does your commit relate to Reid's work on this ticket? And did this one change cure the leak? Reid thinks not? I'd like to work through comment:19 too. Moreover, presumably the commit you did make (in `rebuildCase`) presumably does help something substantially, or you would not have done it (esp given its equivocal effect on other perf tests). But neither the commit message, nor (more importantly) the comment in the code, says what! `DynFlags` perhaps? Please say, so that someone wondering if they can now take it out knows what they have to test against. I'm re-opening pending resolving some of these mysteries. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13426#comment:24 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13426: compile-time memory-usage regression for DynFlags between GHC 8.0 and GHC 8.2 -------------------------------------+------------------------------------- Reporter: hvr | Owner: (none) Type: bug | Status: new Priority: high | Milestone: 8.2.1 Component: Compiler | Version: 8.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Compile-time | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D3399, Wiki Page: | Phab:D3400 -------------------------------------+------------------------------------- Comment (by bgamari): I'll have to defer to Reid on this. I merely merged the patch he provided in response to my request for a fix. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13426#comment:25 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
Moreover, presumably the commit you did make (in `rebuildCase`)
#13426: compile-time memory-usage regression for DynFlags between GHC 8.0 and GHC 8.2 -------------------------------------+------------------------------------- Reporter: hvr | Owner: (none) Type: bug | Status: new Priority: high | Milestone: 8.2.1 Component: Compiler | Version: 8.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Compile-time | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D3399, Wiki Page: | Phab:D3400, Phab:D3421 -------------------------------------+------------------------------------- Changes (by rwbarton): * differential: Phab:D3399, Phab:D3400 => Phab:D3399, Phab:D3400, Phab:D3421 Comment: Yes, I'm not sure what happened here but one of the parts of the patch seems to have been lost in a rebase. Thanks for catching this Simon! presumably does help something substantially, or you would not have done it (esp given its equivocal effect on other perf tests). But neither the commit message, nor (more importantly) the comment in the code, says what! `DynFlags` perhaps? Please say, so that someone wondering if they can now take it out knows what they have to test against. Yes, good point. Together with the missing part of the patch, it reduces the space usage while compiling `DynFlags` by at least a factor of 2. Unfortunately I was unable to construct a simpler test case that shows a similarly dramatic improvement, or diagnose exactly what is so special about `DynFlags`... maybe I'll try again to come up with a perf test case, even if the effect is not as dramatic as for `DynFlags`. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13426#comment:26 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13426: compile-time memory-usage regression for DynFlags between GHC 8.0 and GHC 8.2 -------------------------------------+------------------------------------- Reporter: hvr | Owner: (none) Type: bug | Status: new Priority: high | Milestone: 8.2.1 Component: Compiler | Version: 8.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Compile-time | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D3399, Wiki Page: | Phab:D3400, Phab:D3421 -------------------------------------+------------------------------------- Comment (by rwbarton): Regarding comment:19: this is only needed for the subsequent change to make `coreBindsSize` less strict without introducing new space leaks. It's not needed to get the space usage under control in the first place, for which Phab:D3421 should be sufficient. That said I think it is still worth pursuing that line, I'm just not sure about the timing for 8.2.1. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13426#comment:27 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13426: compile-time memory-usage regression for DynFlags between GHC 8.0 and GHC 8.2 -------------------------------------+------------------------------------- Reporter: hvr | Owner: (none) Type: bug | Status: new Priority: high | Milestone: 8.2.1 Component: Compiler | Version: 8.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Compile-time | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D3399, Wiki Page: | Phab:D3400, Phab:D3421 -------------------------------------+------------------------------------- Comment (by rwbarton): Now I see that Phab:D3421 is also about forcing types that go into a `SimplCont`, so is the situation similar to that of comment:19 where it would be better to force the types when we take them out of the `SimplCont` and put them into the actual program? -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13426#comment:28 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13426: compile-time memory-usage regression for DynFlags between GHC 8.0 and GHC 8.2 -------------------------------------+------------------------------------- Reporter: hvr | Owner: (none) Type: bug | Status: new Priority: high | Milestone: 8.2.1 Component: Compiler | Version: 8.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Compile-time | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D3399, Wiki Page: | Phab:D3400, Phab:D3421 -------------------------------------+------------------------------------- Comment (by simonpj):
would be better to force the types when we take them out of the SimplCont and put them into the actual program?
Yes for the type in the `Stop` continuation; but I think no for the types in `ApplyToTy` continuation, which will certainly be used. I'd force the latter on the way in; but the former only on the way out in `contResultType`. Does that make sense? -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13426#comment:29 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13426: compile-time memory-usage regression for DynFlags between GHC 8.0 and GHC 8.2 -------------------------------------+------------------------------------- Reporter: hvr | Owner: (none) Type: bug | Status: new Priority: high | Milestone: 8.2.1 Component: Compiler | Version: 8.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Compile-time | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D3399, Wiki Page: | Phab:D3400, Phab:D3421 -------------------------------------+------------------------------------- Comment (by rwbarton): After writing comment:28 I got the validate results from Phab:D3421 which show a ~75% increase in allocations when compiling [https://raw.githubusercontent.com/ghc/ghc/master/testsuite/tests/perf/compil... T5631] which involves many very large types; the only change in that commit being to the `App fun arg` case of `simplExprF1`. I see that the `sc_arg_ty` is (almost) always used because it occurs as the type a function is applied to in the output program. But `sc_hole_ty` is only ever used via `contHoleType` and put in a `mkBoringStop`. There is even a Note `[The hole type in ApplyToTy]` which suggests that evaluating `sc_hole_ty` eagerly would be bad. I tried going back to using `substTy` for the `sc_hole_ty` field, while using `simplType` for the `sc_arg_ty` field. This still avoids the space leak when compiling `DynFlags` and also avoids the regression in T5631. So it seems to be the right thing to do. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13426#comment:30 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13426: compile-time memory-usage regression for DynFlags between GHC 8.0 and GHC 8.2 -------------------------------------+------------------------------------- Reporter: hvr | Owner: (none) Type: bug | Status: new Priority: high | Milestone: 8.2.1 Component: Compiler | Version: 8.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Compile-time | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D3399, Wiki Page: | Phab:D3400, Phab:D3421 -------------------------------------+------------------------------------- Comment (by simonpj):
But sc_hole_ty is only ever used via contHoleType and put in a mkBoringStop
Ah, very good! Yes yes. Let's make sure we document all this carefully when we are done :-). Simon -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13426#comment:31 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13426: compile-time memory-usage regression for DynFlags between GHC 8.0 and GHC
8.2
-------------------------------------+-------------------------------------
Reporter: hvr | Owner: (none)
Type: bug | Status: new
Priority: high | Milestone: 8.2.1
Component: Compiler | Version: 8.1
Resolution: | Keywords:
Operating System: Unknown/Multiple | Architecture:
Type of failure: Compile-time | Unknown/Multiple
performance bug | Test Case:
Blocked By: | Blocking:
Related Tickets: | Differential Rev(s): Phab:D3399,
Wiki Page: | Phab:D3400, Phab:D3421
-------------------------------------+-------------------------------------
Comment (by Ben Gamari
#13426: compile-time memory-usage regression for DynFlags between GHC 8.0 and GHC 8.2 -------------------------------------+------------------------------------- Reporter: hvr | Owner: (none) Type: bug | Status: closed Priority: high | Milestone: 8.2.1 Component: Compiler | Version: 8.1 Resolution: fixed | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Compile-time | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D3399, Wiki Page: | Phab:D3400, Phab:D3421 -------------------------------------+------------------------------------- Changes (by bgamari): * status: new => closed * resolution: => fixed Comment: Merged to `ghc-8.2` as f636599165a72a29830341049023651c6fbf38c9. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13426#comment:33 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13426: compile-time memory-usage regression for DynFlags between GHC 8.0 and GHC 8.2 -------------------------------------+------------------------------------- Reporter: hvr | Owner: (none) Type: bug | Status: new Priority: high | Milestone: 8.2.1 Component: Compiler | Version: 8.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Compile-time | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D3399, Wiki Page: | Phab:D3400, Phab:D3421 -------------------------------------+------------------------------------- Changes (by simonpj): * status: closed => new * resolution: fixed => Comment: I'm re-opening because we have not closed the discussion I opened in comment:19. We discussed it on Tuesday and Reid was going to try forcing the type on the way ''out'' of `SimplCont` rather than on the way ''in''. OK, Reid? -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13426#comment:34 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13426: compile-time memory-usage regression for DynFlags between GHC 8.0 and GHC 8.2 -------------------------------------+------------------------------------- Reporter: hvr | Owner: (none) Type: bug | Status: new Priority: high | Milestone: 8.2.1 Component: Compiler | Version: 8.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Compile-time | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D3399, Wiki Page: | Phab:D3400, Phab:D3421 -------------------------------------+------------------------------------- Comment (by rwbarton): I think that is what I've done in the final version of Phab:D3421. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13426#comment:35 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13426: compile-time memory-usage regression for DynFlags between GHC 8.0 and GHC 8.2 -------------------------------------+------------------------------------- Reporter: hvr | Owner: (none) Type: bug | Status: closed Priority: high | Milestone: 8.2.1 Component: Compiler | Version: 8.1 Resolution: fixed | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Compile-time | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D3399, Wiki Page: | Phab:D3400, Phab:D3421 -------------------------------------+------------------------------------- Changes (by simonpj): * status: new => closed * resolution: => fixed Comment: Ah yes, I wasn't looking hard enough. Thanks. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13426#comment:36 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13426: compile-time memory-usage regression for DynFlags between GHC 8.0 and GHC 8.2 -------------------------------------+------------------------------------- Reporter: hvr | Owner: (none) Type: bug | Status: new Priority: high | Milestone: 8.2.1 Component: Compiler | Version: 8.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Compile-time | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D3399, Wiki Page: | Phab:D3400, Phab:D3421 -------------------------------------+------------------------------------- Changes (by simonpj): * status: closed => new * resolution: fixed => Comment: Reopening again, because we still have the Big Hamme {{{ | let sz = coreBindsSize binds , () <- sz `seq` () -- Force it }}} in `SimplCore`. Is this still necessary? Could we just remove it altogether? -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13426#comment:37 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13426: compile-time memory-usage regression for DynFlags between GHC 8.0 and GHC 8.2 -------------------------------------+------------------------------------- Reporter: hvr | Owner: (none) Type: bug | Status: new Priority: high | Milestone: 8.2.1 Component: Compiler | Version: 8.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Compile-time | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D3399, Wiki Page: | Phab:D3400, Phab:D3421 -------------------------------------+------------------------------------- Changes (by asr): * cc: asr (added) -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13426#comment:38 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13426: compile-time memory-usage regression for DynFlags between GHC 8.0 and GHC 8.2 -------------------------------------+------------------------------------- Reporter: hvr | Owner: (none) Type: bug | Status: new Priority: high | Milestone: 8.2.1 Component: Compiler | Version: 8.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Compile-time | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D3399, Wiki Page: | Phab:D3400, Phab:D3421 -------------------------------------+------------------------------------- Comment (by dfeuer): I made the big hammer smaller (see Phab:D3606). Things don't go utterly disastrous, but there are still a few modules over a gigabyte (max somewhere around 1700M). I'm not sure exactly which ones; is there a way to find out without doing a single-threaded build? I'll try to do some profiling and see if I can figure out what's going wrong, exactly. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13426#comment:39 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13426: compile-time memory-usage regression for DynFlags between GHC 8.0 and GHC 8.2 -------------------------------------+------------------------------------- Reporter: hvr | Owner: (none) Type: bug | Status: new Priority: high | Milestone: 8.2.1 Component: Compiler | Version: 8.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Compile-time | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D3399, Wiki Page: | Phab:D3400, Phab:D3421 -------------------------------------+------------------------------------- Comment (by bgamari): I really wish that the build system could record RTS statistics on a per- file basis (e.g. perhaps just emitting a log file per GHC process). That would make collecting this sort of thing much easier. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13426#comment:40 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
I really wish that the build system could record RTS statistics on a
#13426: compile-time memory-usage regression for DynFlags between GHC 8.0 and GHC 8.2 -------------------------------------+------------------------------------- Reporter: hvr | Owner: (none) Type: bug | Status: new Priority: high | Milestone: 8.2.1 Component: Compiler | Version: 8.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Compile-time | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D3399, Wiki Page: | Phab:D3400, Phab:D3421 -------------------------------------+------------------------------------- Comment (by hvr): Replying to [comment:40 bgamari]: per-file basis (e.g. perhaps just emitting a log file per GHC process). That would make collecting this sort of thing much easier. We already emit some extra output (mostly for debugging purposes) to `-dumpdir dir`, so there'd be precedent to use output files for logging data, rather than dump it to stdout only. So one way would be to extend `-Rghc-timing` to optionally take a filename, e.g. `-Rghc-timing=rts- stats.log`; similarly for other statistics David pointed out (e.g. optimiser stats). -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13426#comment:41 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13426: compile-time memory-usage regression for DynFlags between GHC 8.0 and GHC
8.2
-------------------------------------+-------------------------------------
Reporter: hvr | Owner: (none)
Type: bug | Status: new
Priority: high | Milestone: 8.2.1
Component: Compiler | Version: 8.1
Resolution: | Keywords:
Operating System: Unknown/Multiple | Architecture:
Type of failure: Compile-time | Unknown/Multiple
performance bug | Test Case:
Blocked By: | Blocking:
Related Tickets: | Differential Rev(s): Phab:D3399,
Wiki Page: | Phab:D3400, Phab:D3421
-------------------------------------+-------------------------------------
Comment (by Ben Gamari
#13426: compile-time memory-usage regression for DynFlags between GHC 8.0 and GHC 8.2 -------------------------------------+------------------------------------- Reporter: hvr | Owner: (none) Type: bug | Status: new Priority: high | Milestone: 8.2.1 Component: Compiler | Version: 8.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Compile-time | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D3399, Wiki Page: | Phab:D3400, Phab:D3421 -------------------------------------+------------------------------------- Comment (by simonpj): Hang on
in some cases a ways above the ones from 8.0.
But I see no changes in the tessuite in this patch. So what regressed? Or, were all the regressions relative to 8.0, and had already been accepted for 8.2? Which cases in particular regressed wrt 8.0? "Some cases" isn't very explicit! Did removing the `coreBindsSize` improve compiler speed (slightly) becuase of doing less work? -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13426#comment:43 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13426: compile-time memory-usage regression for DynFlags between GHC 8.0 and GHC 8.2 -------------------------------------+------------------------------------- Reporter: hvr | Owner: (none) Type: bug | Status: new Priority: high | Milestone: 8.2.1 Component: Compiler | Version: 8.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Compile-time | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D3399, Wiki Page: | Phab:D3400, Phab:D3421 -------------------------------------+------------------------------------- Comment (by bgamari): dfeuer will need to comment on what specifically "some cases" refers to, but the patch in comment:42 removes some `seq`s which don't appear to be necessary to prevent thunk leaks. I would expect that this did have a positive impact on compiler speed, but in practice this effect [[https://perf.haskell.org/ghc/#revision/2088d0be17dccfa91a4759bdbb20faae77c8dbed|appears]] to be too small to measure. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13426#comment:44 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13426: compile-time memory-usage regression for DynFlags between GHC 8.0 and GHC 8.2 -------------------------------------+------------------------------------- Reporter: hvr | Owner: dfeuer Type: bug | Status: new Priority: high | Milestone: 8.2.2 Component: Compiler | Version: 8.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Compile-time | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D3399, Wiki Page: | Phab:D3400, Phab:D3421 -------------------------------------+------------------------------------- Changes (by bgamari): * owner: (none) => dfeuer * milestone: 8.2.1 => 8.2.2 Comment: dfeuer is going to look into eliminating the need for `coreBindsSize` on every simplifier pass. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13426#comment:45 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13426: compile-time memory-usage regression for DynFlags between GHC 8.0 and GHC 8.2 -------------------------------------+------------------------------------- Reporter: hvr | Owner: dfeuer Type: bug | Status: new Priority: high | Milestone: 8.4.1 Component: Compiler | Version: 8.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Compile-time | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D3399, Wiki Page: | Phab:D3400, Phab:D3421 -------------------------------------+------------------------------------- Changes (by bgamari): * milestone: 8.2.2 => 8.4.1 -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13426#comment:46 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13426: compile-time memory-usage regression for DynFlags between GHC 8.0 and GHC 8.2 -------------------------------------+------------------------------------- Reporter: hvr | Owner: dfeuer Type: bug | Status: new Priority: high | Milestone: 8.4.1 Component: Compiler | Version: 8.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Compile-time | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D3399, Wiki Page: | Phab:D3400, Phab:D3421 -------------------------------------+------------------------------------- Comment (by dfeuer): If I remove the size calculation altogether, allowing unlimited simplifier ticks, then we get performance regressions. This suggests that some of the leaks are still with us, although apparently not nearly as severe as they once were. I'm working on making the size calculation a bit more efficient, but we're going to leave the rest alone for now. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13426#comment:47 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13426: compile-time memory-usage regression for DynFlags between GHC 8.0 and GHC 8.2 -------------------------------------+------------------------------------- Reporter: hvr | Owner: dfeuer Type: bug | Status: new Priority: high | Milestone: 8.4.1 Component: Compiler | Version: 8.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Compile-time | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: #7258 | Differential Rev(s): Phab:D3399, Wiki Page: | Phab:D3400, Phab:D3421 -------------------------------------+------------------------------------- Changes (by dfeuer): * related: => #7258 -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13426#comment:48 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13426: compile-time memory-usage regression for DynFlags between GHC 8.0 and GHC 8.2 -------------------------------------+------------------------------------- Reporter: hvr | Owner: dfeuer Type: bug | Status: new Priority: high | Milestone: 8.6.1 Component: Compiler | Version: 8.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Compile-time | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: #7258 | Differential Rev(s): Phab:D3399, Wiki Page: | Phab:D3400, Phab:D3421 -------------------------------------+------------------------------------- Changes (by bgamari): * milestone: 8.4.1 => 8.6.1 Comment: This won't be fixed for 8.4.1 -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13426#comment:49 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13426: compile-time memory-usage regression for DynFlags between GHC 8.0 and GHC 8.2 -------------------------------------+------------------------------------- Reporter: hvr | Owner: dfeuer Type: bug | Status: new Priority: high | Milestone: 8.6.1 Component: Compiler | Version: 8.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Compile-time | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: #7258 | Differential Rev(s): Phab:D3399, Wiki Page: | Phab:D3400, Phab:D3421 -------------------------------------+------------------------------------- Changes (by gbaz): * cc: gbaz (added) -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13426#comment:50 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13426: compile-time memory-usage regression for DynFlags between GHC 8.0 and GHC 8.2 -------------------------------------+------------------------------------- Reporter: hvr | Owner: dfeuer Type: bug | Status: new Priority: high | Milestone: 8.6.1 Component: Compiler | Version: 8.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Compile-time | Unknown/Multiple performance bug | Test Case: Blocked By: | Blocking: Related Tickets: #7258 | Differential Rev(s): Phab:D3399, Wiki Page: | Phab:D3400, Phab:D3421 -------------------------------------+------------------------------------- Changes (by gbaz): * cc: gbaz (removed) -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13426#comment:51 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
participants (1)
-
GHC