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

Re: [PATCH] issue 1954 - v3

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: 2004-12-14 14:55:12 CET

VK Sameer wrote:
>>>--- subversion/libsvn_wc/update_editor.c (revision 12257)
>>>+++ subversion/libsvn_wc/update_editor.c (working copy)
>>>@@ -1431,10 +1432,12 @@
>>> const svn_wc_entry_t *entry;
>>> svn_node_kind_t kind;
>>> svn_wc_adm_access_t *adm_access;
>>>+ apr_pool_t *subpool;
>>> /* the file_pool can stick around for a *long* time, so we want to use
>>> a subpool for any temporary allocations. */
>>>- apr_pool_t *subpool = svn_pool_create (pool);
>>>+ subpool = svn_pool_create (pool);
>>>+ SVN_ERR (svn_path_check_valid (path, subpool));
> Sorry, I meant to use pool instead. The thought was that this would
> avoid increasing memory usage just before erroring out. Updated in
> patch.

It is not appropriate to optimise for reduced memory usage in the error
condition, since
   + the error condition is the rare case;
   + it was only a small allocation, and one which would have been made in the
common case anyway;
   + the pool memory will get freed soon enough, regardless of which way you do it.

The existing comment explains why we want to use a sub-pool: you saved memory
in the error condition at the expense of potentially using extra memory in the
file's main pool for every successful open, and that pool may persist for a
long time. I say "potentially" because your current implementation of
svn_path_check_valid doesn't actually use pool unless it errors, but its
interface doesn't guarantee this and a subsequent version of it might well do so.

- Julian

To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Dec 14 14:56:48 2004

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.