Re: svn commit: rev 2216 - trunk/subversion/include trunk/subversion/libsvn_client trunk/subversion/clients/cmdline trunk/subversion/tests/clients/cmdline/getopt_tests_data

On Fri, Jun 14, 2002 at 12:53:11PM -0700, Greg Stein wrote:

[ snip a lot of good points ]

i've fixed the other problems, and will commit the changes soon...

> >...
> > +
> > + svn_pool_destroy (subpool);
>
> This subpool covers the whole function. I think you may want to just use the
> caller's pool for the initial entry and hash table, then use the subpool for
> the iteration. A directory with 10k files in it will cause problems with the
> "whole function" subpool.

couldn't i also use the subpool for the svn_wc_entry, apr_stat, and
apr_dir_make? i mean there's no reason to put the allocation in the
parent's pool for those if we don't need them to stick around... the
only things that need to persist through the loop are the dirents and
the apr_hash_index_t, so couldn't i get away with using the parent
pool for svn_io_get_dirents and apr_hash_first, and using the subpool
for everything else, clearing it each time through the loop?

> And in general, a subpool that just wraps what a function does is suspect.
> Subpools are important for loops. There are *some* specific functions where
> we know that its memory is so huge that we want to create/destroy a pool for
> just that function, but it is rare.

makes sense.

thanks for the review,

-garrett

-- 
garrett rooney                    Remember, any design flaw you're 
rooneg@electricjellyfish.net      sufficiently snide about becomes  
http://electricjellyfish.net/     a feature.       -- Dan Sugalski
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org