Ticket #2328 (closed compile-time performance bug: fixed)

Opened 6 months ago

Last modified 2 months ago

Compiling DoCon with 6.8.3 has 3x slow-down compared with 6.8.2

Reported by: simonpj Assigned to: simonpj
Priority: high Milestone: 6.10.1
Component: Compiler Version: 6.8.2
Severity: normal Keywords:
Cc: Difficulty: Unknown
Test Case: Operating System: Unknown/Multiple
Architecture: Unknown/Multiple

Description

Serge reports that "there remains the question: why GHC 6.8.3 release candidate builds DoCon-2.11 considerably slower than ghc-6.8.2 (3 times, as I recall) and needs larger -M memory to build this DoCon".

A 3x slow-down wrt 6.8.2 is quite unexpected.

Simon

Change History

06/04/08 18:51:30 changed by igloo

  • milestone set to 6.8.3.

Here's where I am so far, using http://haskell.org/docon/distrib/2.11/docon-2.11.zip

With 6.8.3 RC:

$ ghc -O -fglasgow-exts -fallow-overlapping-instances -fallow-undecidable-instances -O --make RsePol_.hs -fforce-recomp +RTS -M400M
[...]
$ ghc -O -fglasgow-exts -fallow-overlapping-instances -fallow-undecidable-instances -O -c RsePol_.hs -fforce-recomp -v +RTS -M400M
[...]
*** Parser:
*** Renamer/typechecker:
*** Desugar:
    Result size = 3830
*** Simplify:
    Result size = 2895
    Result size = 2691
    Result size = 2689
    Result size = 2689
*** Specialise:
    Result size = 2689
*** Float out (not lambdas, not constants):
    Result size = 2751
*** Float inwards:
    Result size = 2751
*** Simplify:
Heap exhausted;
Current maximum heap size is 399998976 bytes (381 Mb);

while 6.8.2 goes:

[...]
*** Parser:
*** Renamer/typechecker:
*** Desugar:
    Result size = 3827
*** Simplify:
    Result size = 2899
    Result size = 2701
    Result size = 2699
    Result size = 2699
*** Specialise:
    Result size = 2699
*** Float out (not lambdas, not constants):
    Result size = 2768
*** Float inwards:
    Result size = 2768
*** Simplify:
    Result size = 309049
    Result size = 216314
    Result size = 81507
[...]

(and is also happy with a 200M heap).

06/05/08 02:21:59 changed by simonpj

I believe that the massive (150x!) blow-up when the Simplifier gets going is because instance declarations are inlined bodily. DoCon uses a lot of instances, and the effect is very substantial. As it happens I've been working on a fix, because I've been aware of this for some time, but it's too big a fix to put in 6.8.2.

The change between 6.8.2 and 6.8.3 is mysterious though. I don't understand that.

Simon

06/05/08 05:51:47 changed by igloo

The problem seems to be that this patch:

[MERGED: Inline implication constraints
Ian Lynagh <igloo@earth.li>**20071215163315
 Mon Nov  5 22:08:07 GMT 2007  simonpj@microsoft.com
 
   This patch fixes Trac #1643, where Lennart found that GHC was generating
   code with unnecessary dictionaries.  The reason was that we were getting
   an implication constraint floated out of an INLINE (actually an instance
   decl), and the implication constraint therefore wasn't inlined even 
   though it was used only once (but inside the INLINE).  Thus we were 
   getting:
   
         ic = \d -> <stuff>
         foo = _inline_me_ (...ic...)
   
   Then 'foo' gets inlined in lots of places, but 'ic' now looks a bit 
   big.  
   
   But implication constraints should *always* be inlined; they are just
   artefacts of the constraint simplifier.
   
   This patch solves the problem, by adding a WpInline form to the HsWrap
   type. 

makes more things inlinable, and thus requires more space. Reverting the patch gets us back to 6.8.2 performance. Should I revert it in the 6.8 branch, Simon?

06/05/08 06:43:30 changed by simonpj

Good catch!

I'm not sure about reverting, though, because I think it may worsen runtime performance in quite ordinary programs. I'm inclined to keep this one firmly on the radar, but wait see how my upcoming change to instances affects it. Meanwhile, Serge may have to compile with 6.8.2, which is, I admit, rather unsatisfactory.

Simon

06/05/08 07:34:01 changed by igloo

  • owner set to simonpj.
  • milestone changed from 6.8.3 to 6.10 branch.

07/11/08 06:12:57 changed by simonmar

  • priority changed from normal to high.
  • milestone changed from 6.10 branch to 6.10.1.

Sounds like this is important to fix in 6.10.1.

07/11/08 08:48:25 changed by maeder

probably related to #955

09/15/08 09:30:42 changed by simonpj

  • status changed from new to closed.
  • resolution set to fixed.

I believe this is now fixed. Hurrah!

Simon

09/30/08 08:39:50 changed by simonmar

  • architecture changed from Unknown to Unknown/Multiple.

09/30/08 08:50:56 changed by simonmar

  • os changed from Unknown to Unknown/Multiple.