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

Re: SVN_ERR bypassing "goto cleanup" [was: svn commit: r14643 - ...]

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: 2005-05-21 20:36:39 CEST

Julian Foad wrote:
> Erik Huelsmann wrote:
>> On 5/11/05, Julian Foad <julianfoad@btopenworld.com> wrote:
>>>> - libsvn_wc/questions.c:svn_wc_text_modified_p
>>>> - libsvn_wc/props.c:svn_wc_props_modified_p
>>>> fails to delete a subpool
>>>
>>> I can't see why those functions should be using sub-pools: I can't see any
>>> repetition or very large memory allocations. Can we get rid of their sub-pools?
>>
>> Well, if we're allocating really large chuncks of memory, we sometimes
>> use subpools [...]
>
> Do you think these functions _do_ allocate large amounts of memory?

Here's the log message from when the subpool was introduced:

> ------------------------------------------------------------------------
> r205 | cmpilato | 2001-10-04 01:10:11 +0100 (Thu, 04 Oct 2001) | 23 lines
>
> Sorry, but 10 megs of memory just to run `svn st' on the subversion
> source tree is ridiculous. This brought my test case down below 400k.
>
> * subversion/libsvn_wc/questions.c
>
> (svn_wc_text_modified_p, svn_wc_props_modified_p,
> svn_wc_conflicted_p, svn_wc_has_binary_prop): Use a subpool, and
> destroy it when we're done. For crying out loud, we're returning
> data allocated outside the function. Also, use the SVN_ERR macro.
>
> (contents_identical_p): 80-column formatting.
>
> (timestamps_equal_p): Use SVN_ERR macro.
>
> * subversion/libsvn_wc/status.c
>
> (assemble_status): Copy the entry into the hash's pool.
>
> (svn_wc_statuses): Use a subpool for all non-hash-related
> allocations, destroying it when we're done. Also, use the SVN_ERR
> macro.
> ------------------------------------------------------------------------

I don't understand the comment about "returning data allocated outside the
function", and I still can't see any large allocations in the pool either then
or now in those two functions (svn_wc_text_modified_p,
svn_wc_props_modified_p), except that svn_wc_props_modified_p loads the base
and working sets of properties, which may be largeish. Do we regard two sets
of properties as large enough to warrant a sub-pool? I wouldn't have thought
so, really. Even if in the exceptional case the properties occupy megabytes,
this is only a problem when there is iteration in which case there will be an
iteration sub-pool.

C-Mike or anyone, can you explain or comment? It seems to me that each of
these functions should lose its subpool.

- Julian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sun May 22 03:32:33 2005

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