On Wed, May 14, 2003 at 08:37:33PM -0600, D.J. Heap wrote:
> Scott Collins wrote:
>...
> >In your patch, would it be as effective to not add |scratchpool|, but in
> >the place where you added |svn_pool_clear (scratchpool);|, substitute
> >|svn_pool_clear (subpool);|? But if turns out you must add
> >|scratchpool|, is it better if it comes out of |subpool| rather than
> >|pool|?
>...
> I believe subpool must not be cleared (actually I tried it and boom)
> because the hash iterator is in it?
Right.
Looking at that function, it would be fixed if we follow standard design
policy here:
* things allocated outside a loop are placed into the pool passed to the
function. the caller knows how many times the function will be called and
will deal with handling the passed-in pool properly (clearing and
destroying at the right times).
placing these items into a subpool doesn't help since the peak memory
usage is the same whether you place them directly into the pool or into a
subpool. thus, the policy to use the passed-in pool.
* anything within a loop should use a subpool (sometimes called an "iterator
pool"). the subpool is cleared on each entry to the loop.
Thus: place the svn_fs_paths_changed() can apr_has_first() results into
'pool'. The item and the paths placed into the item also go into the pool
since they need to live past the function-return. The svn_fs_copied_from()
stays in the subpool. The subpool gets cleared on loop-entry.
No scratchpool is needed -- the subpool should have filled that role from
the start.
Cheers,
-g
p.s. separate change here: the 'item' variable declaration would normally
occur inside the block rather than the outermost block. we scope things as
tightly as possible.
p.p.s. the concept of passing in a pre-created hash table, and having the
function fill it in, is also a bit non-standard. it would be best for the
function to allocate the 'changed' hash table itself and return it.
--
Greg Stein, http://www.lyra.org/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu May 15 09:14:42 2003