[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: Sub-pools for recursion

From: Justin Erenkrantz <justin_at_erenkrantz.com>
Date: 2003-11-28 19:43:20 CET

--On Friday, November 28, 2003 2:35 PM +0000 Julian Foad
<julianfoad@btopenworld.com> wrote:

> Should the sub-pool be reserved for either one purpose or the other? If so,
> should we introduce a second sub-pool for the other purpose, as I think is
> done elsewhere? In practice, there is probably not a great deal of memory
> being used here, but I would like to determine a "best practice".

No, I don't think they should be separated. Each pool takes ~8KB, so if it
isn't going to use more than that (which isn't likely in this case), then
creating separate pools is a waste.

> Since the recursive call is made from within the iteration, there is really
> no need for a separate recursion sub-pool in this case, so I think this
> function should regard its sub-pool as an iteration sub-pool. Everything
> that is not local to the iteration should be allocated in the passed-in
> "pool". Does that sound right?

Yes.

> I believe there are definite mis-uses of both "pool" and "sub-pool" in the
> function. Here is an abbreviated and annotated version of the function; all
> comments are mine:
>
> copy_versioned_files (..., apr_pool_t *pool)
> {
> apr_pool_t *subpool = svn_pool_create (pool);
>
> SVN_ERR (svn_wc_adm_probe_open (..., pool)); /* OK: needed
> throughout */
> err = svn_wc_entry (..., subpool); /* ### Wrong: needed
> throughout */

entry is only used once at first - the subpool is indeed safe. Then, look at
when we obtain entry in the svn_node_file path with subpool again.

> if (entry)
> {
> SVN_ERR (svn_io_stat (&finfo, ..., subpool)); /* Safe (used only
> once), but might as well go in "pool" */

svn_io_stat is a ridiculous memory hog. It needs to be in the subpool.

> SVN_ERR (svn_subst_build_keywords
> (...,
> apr_psprintf (pool, ...), /* Safe but should go
> in "subpool" */

Agreed. That printf is very recent (I had to update my WC to get it), so
that'll probably explain why it has improper memory usage.

I'm also concerned that adm_access could be used after it is closed in
copy_versioned_files. Philip? -- justin

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Nov 28 19:44:39 2003

This is an archived mail posted to the Subversion Dev mailing list.