You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Greg Hudson <gh...@MIT.EDU> on 2007/01/08 03:58:34 UTC
Re: [PATCH] Destroy the APR subpool before every successful
returnstatement
On Sun, 2007-01-07 at 13:00 +0100, Erik Huelsmann wrote:
> Yes it is. If we were not intending to destroy the memory, why use a
> subpool at all?
If there's no loop, the use of a subpool is pointless and should be
excised.
> Non-destruction of pools is valid when returning *in the error-case*,
> because we assume that the error will be handled and a parent pool
> will be cleared somewhere higher up the call chain. But for the
> non-error return, we neither assume nor actually do something like it.
We leave memory allocated in the normal return case all the time.
If high memory use due to call fanout is an issue (and it pretty much
never is), it is an issue for the caller, not the callee.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Destroy the APR subpool before every successful returnstatement
Posted by Daniel Rall <dl...@collab.net>.
On Mon, 08 Jan 2007, Greg Hudson wrote:
> On Mon, 2007-01-08 at 13:36 +0100, Erik Huelsmann wrote:
> > ... yes, except that these functions don't get a pool passed in: they
> > create a subpool inside a pool which they get in through a baton. At
> > the end, they need to destroy the subpool, because if they don't,
> > pools will be created iteratively.
>
> Ah, good catch.
>
> Yes, the functions need to clear the subpool in all non-error cases, and
> (if I haven't missed something about it) the patch should be applied.
+1
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Destroy the APR subpool before every successful
returnstatement
Posted by Greg Hudson <gh...@MIT.EDU>.
On Mon, 2007-01-08 at 13:36 +0100, Erik Huelsmann wrote:
> ... yes, except that these functions don't get a pool passed in: they
> create a subpool inside a pool which they get in through a baton. At
> the end, they need to destroy the subpool, because if they don't,
> pools will be created iteratively.
Ah, good catch.
Yes, the functions need to clear the subpool in all non-error cases, and
(if I haven't missed something about it) the patch should be applied.
The callback signatures should also, in theory, be modified to take a
pool (we recognized some time ago that baton pools are not a good
substitute for argument pools). But if that requires making an API
change it may not be worth the pain.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Destroy the APR subpool before every successful returnstatement
Posted by Erik Huelsmann <eh...@gmail.com>.
On 1/8/07, Greg Hudson <gh...@mit.edu> wrote:
> On Sun, 2007-01-07 at 13:00 +0100, Erik Huelsmann wrote:
> > Yes it is. If we were not intending to destroy the memory, why use a
> > subpool at all?
>
> If there's no loop, the use of a subpool is pointless and should be
> excised.
Well, you are correct in the usual case, but ...
> > Non-destruction of pools is valid when returning *in the error-case*,
> > because we assume that the error will be handled and a parent pool
> > will be cleared somewhere higher up the call chain. But for the
> > non-error return, we neither assume nor actually do something like it.
>
> We leave memory allocated in the normal return case all the time.
... yes, except that these functions don't get a pool passed in: they
create a subpool inside a pool which they get in through a baton. At
the end, they need to destroy the subpool, because if they don't,
pools will be created iteratively.
> If high memory use due to call fanout is an issue (and it pretty much
> never is), it is an issue for the caller, not the callee.
Except that this caller doesn't know the callee even uses a pool...
You could argue this should be rewritten, but that's much more
code-churn than the current solution. As long as we don't rewrite to
put iteration pools in the baton, I'm all for this patch.
bye,
Erik.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org