GHC: Ticket #5205: Control.Monad.forever leaks space
http://ghc.haskell.org/trac/ghc/ticket/5205
<p>
The attached program, compiled with GHC 7.0.3, uses up all the memory. It runs in a constant space when compiled with GHC 6.12.3.
</p>
en-usGHChttp://ghc.haskell.org/trac/ghc/chrome/site/ghc_logo.png
http://ghc.haskell.org/trac/ghc/ticket/5205
Trac 1.0.1akioThu, 19 May 2011 06:29:50 GMTattachment set
http://ghc.haskell.org/trac/ghc/ticket/5205
http://ghc.haskell.org/trac/ghc/ticket/5205
<ul>
<li><strong>attachment</strong>
set to <em>forever.hs</em>
</li>
</ul>
<p>
test case
</p>
TicketiglooSun, 29 May 2011 00:12:54 GMTpriority changed; owner, milestone set
http://ghc.haskell.org/trac/ghc/ticket/5205#comment:1
http://ghc.haskell.org/trac/ghc/ticket/5205#comment:1
<ul>
<li><strong>owner</strong>
set to <em>igloo</em>
</li>
<li><strong>priority</strong>
changed from <em>normal</em> to <em>highest</em>
</li>
<li><strong>milestone</strong>
set to <em>7.2.1</em>
</li>
</ul>
<p>
Thanks for the report. I'll take a look.
</p>
TicketliyangThu, 02 Jun 2011 04:03:50 GMTcc set
http://ghc.haskell.org/trac/ghc/ticket/5205#comment:2
http://ghc.haskell.org/trac/ghc/ticket/5205#comment:2
<ul>
<li><strong>cc</strong>
<em>hackage.haskell.org@…</em> added
</li>
</ul>
TicketsimonmarThu, 09 Jun 2011 10:12:49 GMTowner changed
http://ghc.haskell.org/trac/ghc/ticket/5205#comment:3
http://ghc.haskell.org/trac/ghc/ticket/5205#comment:3
<ul>
<li><strong>owner</strong>
changed from <em>igloo</em> to <em>simonpj</em>
</li>
</ul>
<p>
Ok, we looked at this, and it turns out that 6.12.3 desugars <tt>forever</tt> differently: in 6.12, a local recursive <tt>let</tt> was introduced, which meant that <tt>forever</tt> could be inlined (and hence specialised) at every call site, whereas in 7.0 the desugarer leaves the function as a top-level recursive function which cannot be inlined.
</p>
<p>
The solution is to add an <tt>INLINABLE</tt> pragma for <tt>forever</tt>, which will allow it to be specialised at a call site.
</p>
TicketsimonpjThu, 09 Jun 2011 19:51:37 GMTowner changed
http://ghc.haskell.org/trac/ghc/ticket/5205#comment:4
http://ghc.haskell.org/trac/ghc/ticket/5205#comment:4
<ul>
<li><strong>owner</strong>
changed from <em>simonpj</em> to <em>igloo</em>
</li>
</ul>
<p>
I've made 'forever' INLINABLE, and added a SPECIALISE pragma in GHC.ST.
</p>
<pre class="wiki">commit ae10342b49b95393b09ffee8df8c847409699968
Author: Simon Peyton Jones <simonpj@microsoft.com>
Date: Thu Jun 9 20:44:21 2011 +0100
Make 'forever' inlinable (fixes Trac #5205)
See Note [Make forever INLINABLE] in Control.Monad
>---------------------------------------------------------------
Control/Monad.hs | 18 ++++++++++++++++++
GHC/ST.lhs | 4 ++++
2 files changed, 22 insertions(+), 0 deletions(-)
</pre><p>
That fixes the bug, but only with <tt>-O</tt>. Without <tt>-O</tt> you still get the leak, and I don't really think its unreasonable. You have
</p>
<pre class="wiki">x = forever (return ())
</pre><p>
which expands to
</p>
<pre class="wiki">x = return () >> return () >> return () >> ....etc....
</pre><p>
If <tt>(>>)</tt> was expensive when applied to two args, then it'd be right to hang onto the computed result. In the case of IO it isn't expensive, and it's best to recompute (and save space) but only the optimiser can reveal that.
</p>
<p>
Ian: can you add a perf/ test please, then close?
</p>
<p>
Simon
</p>
TicketiglooSun, 19 Jun 2011 20:52:42 GMTpriority changed
http://ghc.haskell.org/trac/ghc/ticket/5205#comment:5
http://ghc.haskell.org/trac/ghc/ticket/5205#comment:5
<ul>
<li><strong>priority</strong>
changed from <em>highest</em> to <em>normal</em>
</li>
</ul>
TicketsimonpjMon, 20 Jun 2011 08:53:43 GMT
http://ghc.haskell.org/trac/ghc/ticket/5205#comment:6
http://ghc.haskell.org/trac/ghc/ticket/5205#comment:6
<p>
Ian: all we need is a perf test for this, and we can close it.
</p>
TicketiglooSun, 26 Jun 2011 16:54:08 GMTstatus changed; resolution set
http://ghc.haskell.org/trac/ghc/ticket/5205#comment:7
http://ghc.haskell.org/trac/ghc/ticket/5205#comment:7
<ul>
<li><strong>status</strong>
changed from <em>new</em> to <em>closed</em>
</li>
<li><strong>resolution</strong>
set to <em>fixed</em>
</li>
</ul>
<p>
Test added.
</p>
TicketedskoThu, 16 Aug 2012 12:56:46 GMTstatus changed; owner, resolution deleted
http://ghc.haskell.org/trac/ghc/ticket/5205#comment:8
http://ghc.haskell.org/trac/ghc/ticket/5205#comment:8
<ul>
<li><strong>owner</strong>
<em>igloo</em> deleted
</li>
<li><strong>status</strong>
changed from <em>closed</em> to <em>new</em>
</li>
<li><strong>resolution</strong>
<em>fixed</em> deleted
</li>
</ul>
<p>
I have an example with a space leak (according to "+RTS -hy") due to "forever" when compiled with "-prof -fauto-all -O1" and when compiled with "-prof -fprof-auto -O2", but not when compiled with "-prof -fprof-auto -O0" for some bizarre reason. When I remove the "-prof -fprof-auto" and profile with "+RTS -hT" the space leak disappears for any optimization level.
</p>
<p>
I have tried to minimize the example but failed so far, unfortunately. However, the following alternative definition of "forever" doesn't have a space leak with any compiler flags:
</p>
<pre class="wiki">forever a = let a' = a >> a' in a'
</pre><p>
Is there a good reason why this definition is not used? It seems less reliant on optimization to avoid the space leak.
</p>
TicketsimonpjThu, 16 Aug 2012 13:37:31 GMTowner, difficulty set
http://ghc.haskell.org/trac/ghc/ticket/5205#comment:9
http://ghc.haskell.org/trac/ghc/ticket/5205#comment:9
<ul>
<li><strong>owner</strong>
set to <em>igloo</em>
</li>
<li><strong>difficulty</strong>
set to <em>Unknown</em>
</li>
</ul>
<p>
Actually that makes a lot of sense. Ian or Paolo, would you like to change the definition of 'forever' as edsko suggests, and check it's ok with your performance test.
</p>
<p>
Ian: you say that you added a test, but you didn't say what the test is on this ticket, so maybe you can do that too?
</p>
<p>
(Thinking about it, I'm really not sure what difference it makes to make the current recursive defn of <tt>forever</tt> INLINABLE. The comment doesn't explain! But if we change the definition it becomes moot anyway.)
</p>
TicketpcapriottiThu, 16 Aug 2012 15:06:30 GMTowner changed
http://ghc.haskell.org/trac/ghc/ticket/5205#comment:10
http://ghc.haskell.org/trac/ghc/ticket/5205#comment:10
<ul>
<li><strong>owner</strong>
changed from <em>igloo</em> to <em>pcapriotti</em>
</li>
</ul>
TicketiglooThu, 16 Aug 2012 15:13:27 GMTtestcase set
http://ghc.haskell.org/trac/ghc/ticket/5205#comment:11
http://ghc.haskell.org/trac/ghc/ticket/5205#comment:11
<ul>
<li><strong>testcase</strong>
set to <em>T5205</em>
</li>
</ul>
TicketKhudyakovFri, 17 Aug 2012 14:18:40 GMTcc changed
http://ghc.haskell.org/trac/ghc/ticket/5205#comment:12
http://ghc.haskell.org/trac/ghc/ticket/5205#comment:12
<ul>
<li><strong>cc</strong>
<em>alexey.skladnoy@…</em> added
</li>
</ul>
TicketsimonmarMon, 20 Aug 2012 11:03:26 GMTpriority, milestone changed
http://ghc.haskell.org/trac/ghc/ticket/5205#comment:13
http://ghc.haskell.org/trac/ghc/ticket/5205#comment:13
<ul>
<li><strong>priority</strong>
changed from <em>normal</em> to <em>high</em>
</li>
<li><strong>milestone</strong>
changed from <em>7.2.1</em> to <em>7.6.1</em>
</li>
</ul>
<p>
Moving to an active milestone.
</p>
TicketpcapriottiMon, 20 Aug 2012 21:28:46 GMTstatus changed
http://ghc.haskell.org/trac/ghc/ticket/5205#comment:14
http://ghc.haskell.org/trac/ghc/ticket/5205#comment:14
<ul>
<li><strong>status</strong>
changed from <em>new</em> to <em>merge</em>
</li>
</ul>
<p>
Pushed:
</p>
<pre class="wiki">commit f55f5574c12ff8dfe57994219eee0702ac8aba2e
Author: Paolo Capriotti <p.capriotti@gmail.com>
Date: Mon Aug 20 16:35:38 2012 +0100
Improve definition of forever (#5205)
The previous implementation was:
forever a = a >> forever a
which can create a space leak in some cases, even with optimizations.
The current implementation:
forever a = let a' = a >> a' in a'
prevents repeated thunk allocations by creating a single thunk for the
final result, even without optimizations.
</pre><p>
I also removed the note and a <tt>SPECIALISE</tt> pragma in GHC.ST.
</p>
TicketpcapriottiSat, 01 Sep 2012 15:50:32 GMTstatus changed; resolution set
http://ghc.haskell.org/trac/ghc/ticket/5205#comment:15
http://ghc.haskell.org/trac/ghc/ticket/5205#comment:15
<ul>
<li><strong>status</strong>
changed from <em>merge</em> to <em>closed</em>
</li>
<li><strong>resolution</strong>
set to <em>fixed</em>
</li>
</ul>
<p>
Merged as <a class="changeset" href="http://ghc.haskell.org/trac/ghc/changeset/ef4218994742e8400a48b4d6e1ae7e6b67650dc4/ghc" title="Fix missing case in coVarsOfTcCo
Reported by Ganesh, Trac #7178. Fix is ...">ef4218994742e8400a48b4d6e1ae7e6b67650dc4</a>.
</p>
Ticket