I don't see anything wrong with this patch. The idea is definitely sound.
Needs another pair of eyes, but on the whole I recommend we commit it.
Yoshiki Hayashi wrote:
>While I was investigating issue 602 (importing large tree
>bloats memory), I found that trace editor never frees memory
>before it dies. The directory with compiled Linux kernel
>and source contains about 12000 directory and files so that
>amounts to about 12000 * (size of dir_baton or file_baton +
>length of path). I don't think it uses much memory but it
>would nicer not to consume memory linear to the number of
>files and directories in a tree. Here's a patch to create
>and destroy subpool based on the implementation of
>libsvn_ra_local/commit_editor.c
>
>Implement freeing directory button using reference counting.
>
>* trace-commit.c
>
>(struct dir_baton): New member, subpool and ref_count.
>(decrement_dir_ref_count): New function, copied from
> libsvn_ra_local/commit_editor.
>(open_root): Create subpool corresponding the directory. Initialize refcount.
>(add_directory): Create subpool. Initialize refcount. Increment parent's
> refcount.
>(open_directory): Ditto.
>(close_directory): Call decrement_dir_ref_count.
>(clsoe_file): Ditto.
>(add_file): Increase parent's refcount.
>(open_file): Ditto.
>
>Index: ./subversion/clients/cmdline/trace-commit.c
>===================================================================
>--- ./subversion/clients/cmdline/trace-commit.c
>+++ ./subversion/clients/cmdline/trace-commit.c Thu Jan 24 13:39:53 2002
>@@ -51,6 +51,8 @@
> svn_stringbuf_t *path;
> svn_boolean_t added;
> svn_boolean_t prop_changed;
>+ apr_pool_t *subpool;
>+ int ref_count;
> };
>
>
>@@ -66,14 +68,44 @@
>
>
> static svn_error_t *
>+decrement_dir_ref_count (struct dir_baton *db)
>+{
>+ if (db == NULL)
>+ return SVN_NO_ERROR;
>+
>+ db->ref_count--;
>+
>+ /* Check to see if *any* child batons still depend on this
>+ directory's pool. */
>+ if (db->ref_count == 0)
>+ {
>+ struct dir_baton *dbparent = db->parent_dir_baton;
>+
>+ /* Destroy all memory used by this baton, including the baton
>+ itself! */
>+ svn_pool_destroy (db->subpool);
>+
>+ /* Tell your parent that you're gone. */
>+ SVN_ERR (decrement_dir_ref_count (dbparent));
>+ }
>+
>+ return SVN_NO_ERROR;
>+}
>+
>+static svn_error_t *
> open_root (void *edit_baton, svn_revnum_t base_revision, void **root_baton)
> {
>+ apr_pool_t *subpool;
> struct edit_baton *eb = edit_baton;
>- struct dir_baton *rb = apr_pcalloc (eb->pool, sizeof (*rb));
>+ struct dir_baton *rb;
>
>+ subpool = svn_pool_create (eb->pool);;
>+ rb = apr_pcalloc (eb->pool, sizeof (*rb));
> rb->edit_baton = eb;
> rb->parent_dir_baton = NULL;
> rb->path = eb->initial_path;
>+ rb->subpool = subpool;
>+ rb->ref_count = 1;
>
> *root_baton = rb;
>
>@@ -102,16 +134,21 @@
> svn_revnum_t copyfrom_revision,
> void **child_baton)
> {
>+ apr_pool_t *subpool;
> struct dir_baton *parent_d = parent_baton;
>- struct dir_baton *child_d
>- = apr_pcalloc (parent_d->edit_baton->pool, sizeof (*child_d));
>+ struct dir_baton *child_d;
>
>+ subpool = svn_pool_create (parent_d->subpool);
>+ child_d = apr_pcalloc (parent_d->edit_baton->pool, sizeof (*child_d));
> child_d->edit_baton = parent_d->edit_baton;
> child_d->parent_dir_baton = parent_d;
> child_d->path = svn_stringbuf_dup (parent_d->path,
> child_d->edit_baton->pool);
> svn_path_add_component (child_d->path, name);
> child_d->added = TRUE;
>+ child_d->subpool = subpool;
>+ child_d->ref_count = 1;
>+ parent_d->ref_count++;
>
> printf ("Adding %s\n", child_d->path->data);
> *child_baton = child_d;
>@@ -126,15 +163,20 @@
> svn_revnum_t base_revision,
> void **child_baton)
> {
>+ apr_pool_t *subpool;
> struct dir_baton *parent_d = parent_baton;
>- struct dir_baton *child_d
>- = apr_pcalloc (parent_d->edit_baton->pool, sizeof (*child_d));
>+ struct dir_baton *child_d;
>
>+ subpool = svn_pool_create (parent_d->subpool);
>+ child_d = apr_pcalloc (parent_d->edit_baton->pool, sizeof (*child_d));
> child_d->edit_baton = parent_d->edit_baton;
> child_d->parent_dir_baton = parent_d;
> child_d->path = svn_stringbuf_dup (parent_d->path,
> child_d->edit_baton->pool);
> svn_path_add_component (child_d->path, name);
>+ child_d->subpool = subpool;
>+ child_d->ref_count = 1;
>+ parent_d->ref_count++;
>
> *child_baton = child_d;
>
>@@ -153,6 +195,7 @@
> if (db->prop_changed)
> printf ("Sending %s\n", db->path->data);
>
>+ decrement_dir_ref_count (db);
> return SVN_NO_ERROR;
> }
>
>@@ -168,7 +211,8 @@
> fb->path->data);
> else
> printf ("Sending %s\n", fb->path->data);
>-
>+
>+ decrement_dir_ref_count (fb->parent_dir_baton);
> return SVN_NO_ERROR;
> }
>
>@@ -218,6 +262,8 @@
> child_fb->binary = FALSE;
> *file_baton = child_fb;
>
>+ parent_d->ref_count++;
>+
> return SVN_NO_ERROR;
> }
>
>@@ -238,6 +284,8 @@
> svn_path_add_component (child_fb->path, name);
>
> *file_baton = child_fb;
>+
>+ parent_d->ref_count++;
>
> return SVN_NO_ERROR;
> }
>
>
--
Brane Čibej <brane_at_xbc.nu> http://www.xbc.nu/brane/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Oct 21 14:36:59 2006