Eric Gillespie <epg@pretzelnet.org> writes:
> This passes the smoke test of allowing my massive import. If it
> passes the test suite and no one objects, i'll commit this.
>
> When committing 89482 files, the server process shoots from 20 MB to
> 69 MB in write_final_changed_path_info. The first 39 MB of that comes
> from fetch_all_changes, the last 10 from
> write_final_changed_path_info's own loop. By fixing these functions'
> pool usage, the 39 MB is reduced to 15, the 10 MB to nearly 0.
>
> * subversion/libsvn_fs_fs/fs_fs.c
> (fetch_all_changes): Allocate change in iterpool, and clear iterpool
> before allocating another change at the end of the loop.
>
> (write_final_changed_path_info): Use an iterpool for the loop.
>
> Index: subversion/libsvn_fs_fs/fs_fs.c
> ===================================================================
> --- subversion/libsvn_fs_fs/fs_fs.c (revision 11677)
> +++ subversion/libsvn_fs_fs/fs_fs.c (working copy)
> @@ -2042,7 +2042,7 @@
> /* Read in the changes one by one, folding them into our local hash
> as necessary. */
>
> - SVN_ERR (read_change (&change, file, pool));
> + SVN_ERR (read_change (&change, file, iterpool));
>
> while (change)
> {
> @@ -2081,10 +2081,10 @@
> }
> }
>
> - SVN_ERR (read_change (&change, file, pool));
> -
> /* Clear the per-iteration subpool. */
> svn_pool_clear (iterpool);
> +
> + SVN_ERR (read_change (&change, file, iterpool));
> }
You could move the initial read_change() call right up against the
'while' loop, and add a comment to indicate that that first call is
really the start of the loop -- the initialization stage of the loop,
in a sense.
But I think a better option would be to switch to a 'do-while' loop,
and put the single read_change() call at the start of the loop. That
way we don't have duplicate calls to read_change(), *and* we get to
use our standard iteration pool pattern, in which we clear the pool at
the beginning of the loop instead of the end.
> /* Destroy the per-iteration subpool. */
> @@ -3276,7 +3276,8 @@
> apr_hash_t *changed_paths, *copyfrom_cache = apr_hash_make (pool);
> apr_off_t offset;
> apr_hash_index_t *hi;
> -
> + apr_pool_t *iterpool = svn_pool_create (pool);
> +
> SVN_ERR (get_file_offset (&offset, file, pool));
>
> SVN_ERR (svn_fs_fs__txn_changes_fetch (&changed_paths, fs, txn_id,
> @@ -3293,6 +3294,8 @@
> void *val;
> apr_ssize_t keylen;
>
> + svn_pool_clear (iterpool);
> +
> apr_hash_this (hi, &key, &keylen, &val);
> change = val;
>
> @@ -3304,7 +3307,7 @@
> if ((change->change_kind != svn_fs_path_change_delete) &&
> (! svn_fs_fs__id_txn_id (id)))
> {
> - SVN_ERR (svn_fs_fs__get_node_revision (&noderev, fs, id, pool));
> + SVN_ERR (svn_fs_fs__get_node_revision (&noderev, fs, id, iterpool));
>
> /* noderev has the permanent node-id at this point, so we just
> substitute it for the temporary one. */
> @@ -3315,7 +3318,7 @@
> copyfrom = apr_hash_get (copyfrom_cache, key, APR_HASH_KEY_STRING);
>
> /* Write out the new entry into the final rev-file. */
> - SVN_ERR (write_change_entry (file, key, change, copyfrom, pool));
> + SVN_ERR (write_change_entry (file, key, change, copyfrom, iterpool));
> }
>
> *offset_p = offset;
Oops, where's the apr_pool_destroy(iterpool) call?
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Nov 1 19:27:41 2004