Re: svn commit: rev 1982 - branches/issue-654-dev/subversion/include branches/issue-654-dev/subversion/libsvn_fs

On Mon, May 20, 2002 at 01:48:10PM -0500, cmpilato@tigris.org wrote:
>...
> +++ branches/issue-654-dev/subversion/libsvn_fs/txn-table.c Mon May 20 13:48:08 2002
>...
> @@ -273,21 +321,38 @@
> /* Build a null-terminated array of keys in the transactions table. */
> for (db_err = cursor->c_get (cursor,
> svn_fs__result_dbt (&key),
> - svn_fs__nodata_dbt (&value),
> + svn_fs__result_dbt (&value),
> DB_FIRST);
> db_err == 0;
> db_err = cursor->c_get (cursor,
> svn_fs__result_dbt (&key),
> - svn_fs__nodata_dbt (&value),
> + svn_fs__result_dbt (&value),
> DB_NEXT))
> {

This loop requires an iteration subpool. The current implementation reads
and parses skels for *every* transaction in the table (committed or
uncommitted).

>...
> + /* Parse TRANSACTION skel */
> + txn_skel = svn_fs__parse_skel (value.data, value.size, trail->pool);
> + if (! txn_skel)
> + return svn_fs__err_corrupt_txn
> + (fs, apr_pstrmemdup (pool, key.data, key.size));
> +
> + /* Convert skel to native type. */
> + SVN_ERR (svn_fs__parse_transaction_skel (&txn, txn_skel, trail->pool));

I think it might be a simple answer of using an iteration subpool for the
skel parsing and transaction parse. The DBT in the cursor will get reused,
so we should be okay there.

> + /* If this is a immutable "committed" transaction, ignore it. */
> + if (is_committed (txn))
> + continue;

And make sure the pool gets cleared, even when the continue step is used.

Looking at this function, it also seems we should be using an
apr_array_header_t rather than manually constructing and returning a list of
char* values.

Cheers,
-g

-- 
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