From: Leopold Toetsch <[EMAIL PROTECTED]>
Date: Fri, 03 Feb 2006 14:49:00 +0100
Bob Rogers wrote:
> Worse, the closed-over frame is leaked entirely. (Is this what the
> "obviously leaks memory" comment in src/register.c is talking about, or
> are there other cases of leakage?) But I think I have a handle on
> what's causing this, and hope to propose a fix shortly.
Yep re comment. It's probably just a matter of setting the initial
ctx->ref_count to 1. A context is either de-allocated immediately, if
the sub returns via RetContinuation (re_use := 1 in src/register.c:500)
or when the (ret)continuation is destroyed. The latter case needs a
proper ref_count setting.
Turns out that the leak is due to C<newclosure>, which causes the
frame's ref_count to jump immediately from 0 to 2. One increment is due
to the RetContinuation => Continuation promotion, and the other is added
by parrot_new_closure to reflect the ref from the closure. So the
essence of the fix is to create a Closure:destroy that calls
Parrot_free_context. But I think it would be better to go further; more
on that below.
I'm pretty sure that frames left via exception are also leaking
currently, which would also be covered by above strategy.
leo
I'll take a look at that, then.
From: Leopold Toetsch <[EMAIL PROTECTED]>
Date: Fri, 3 Feb 2006 17:20:22 +0100
On Feb 3, 2006, at 15:49, Nicholas Clark wrote:
> On Tue, Jan 31, 2006 at 02:01:42PM +0100, Leopold Toetsch wrote:
>> Limiting the callframe range, where the continuation can go. Currently
>> creating a continuation is rather expensive . . .
>
> Could this be done lazily? . . .
The 'rather' expensive thing is basically:
while (cont)
cont->vtable = Parrot_base_vtables[enum_class_Continuation]
cont = cont->caller
i.e. placing a new vtable into the whole call chain. Setting some flag
or whatever wouldn't be simpler. The only problem with above is that
it's O(n) in call depth. Therefore the idea of creating 'limited
continuations' that are only allowed to 'jump' between defined places
in the call chain.
No - I don't think that this can be done lazily.
> Nicholas Clark
leo
If I understand correctly, this O(n) step is necessary because the new
Continuation might be used to jump back down the call chain. In that
case, it would be necessary to return more than once through these
contexts, for which RetContinuation doesn't work, being a "use once and
recycle from_ctx immediately" version of Continuation. For this reason,
there are also a handful of other places in the code that must change
RetContinuation to Continuation because RetContinuation is too
aggressive about recycling contexts that might still be referenced.
So suppose we turn RetContinuation into a non-agressive "use once and
maybe recycle" continuation. This could be done by making
RetContinuation use and obey the Parrot_Context ref_count field (and
making sure everybody else does too). In other words, RetContinuation
would drop its context refs after being called, but would only free the
"from" context if doing so made the ref count drop to zero (i.e. the
normal Parrot_free_context thing). We should also provide an explicit
"invalidate" operation that causes the context refs to be dropped
immediately; this would be useful for all continuation classes.
Then, in order to implement a stack-unwinding control structure, one
can create a new RetContinuation in frame X, without triggering
continuation promotion on X's call chain. The new RetContinuation gets
passed down (or stored in a lexical), and is either used, or explicitly
invalidated before exiting the frame; either way, X's ref count drops
back down. Then, returning from frame X (still via its RetContinuation)
would recycle it normally [1].
There are also a number of side benefits:
1. In the normal call/return case for which RetContinuation was
designed, "from" contexts would still get recycled immediately.
2. Creating a Continuation would still provide the desired "return
many" behavior, albeit still with the O(n) call-chain promotion cost,
but the extra expense would be optional. Presumably, language
implementors will know which kind of continuation they need; if they
don't, then we can't help them anyway.
3. If a Closure or RetContinuation is created, the existing
RetContinuation chain would still work, decrementing the ref_count but
without necessarily freeing the context [2].
4. Most of the continuation promotions and the "re_use" parameter to
Parrot_free_context can be made to go away. (IMHO, this will be easier
to maintain, as Parrot hackers won't be expected to figure out whether a
context could be referenced from elsewhere.)
5. Some other context memory leaks should be easier to fix. For
instance, I notice a few cases of:
cont->vtable = Parrot_base_vtables[enum_class_Continuation];
caller->ref_count++;
These promotions are done without first checking whether the continution
is a RetContinuation; if not, the ref_count increment will leak the
context.
6. Last but not least, "use once" with explicit invalidation is a
useful thing to provide for supporting HLL semantics. Indeed, I
discovered the original "Accept return values via a Continuation"
problem when trying to implement non-local exits in my Common Lisp
compiler [3]. I would also like to be able enforce the rule that one
can only exit from a block once; a non-aggressive RetContinuation would
not only support this nicely, it would also recycle the block's frame
that much sooner.
Sound good? Unless I've missed something, this seems like a win
across the board . . .
-- Bob Rogers
http://rgrjr.dyndns.org/
[1] Unless of course there are closures that reference this frame. I
am reluctant to suggest that it be possible to invalidate a
closure. Maybe if there were a separate DownwardClosure class?
[2] I think this would also fix a bug. Closure:invoke promotes the
return continuation of the :outer context, but not any other
continuation. I assume this is just to keep the context alive so
that the outer_ctx chain can be traversed, but it means that
invalidate_retc_context on a deeper frame will stop too soon. If
RetContinuation was less aggressive, then Closure:invoke wouldn't
have to promote at all.
[3] Which ought to be ready for release soon (though I've said that
before).
