You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Greg Stein <gs...@gmail.com> on 2009/02/14 01:59:40 UTC
Re: svn commit: r35813 - branches/explore-wc/subversion/libsvn_wc
On Wed, Feb 11, 2009 at 18:15, Hyrum K. Wright <hy...@hyrumwright.org> wrote:
>...
> +++ branches/explore-wc/subversion/libsvn_wc/entries.c Wed Feb 11 09:15:15 2009 (r35813)
>...
> +/* Select all the rows from actual_node table in WC_DB and put them into
> + *NODES allocated in RESULT_POOL. */
"all" ?!
Please add a TODO about fixing that... we don't want to always read
all rows. Our wc, client, and other code *does* tend to consume O(#
committables), but in almost all other cases, it doesn't try to hold
information about "all nodes" in a working copy.
> +static svn_error_t *
> +fetch_actual_nodes(apr_hash_t **nodes,
> + svn_sqlite__db_t *wc_db,
> + apr_pool_t *scratch_pool,
> + apr_pool_t *result_pool)
> +{
In wc_db.h and elsewhere, I've *always* declared these as
"result_pool" first, then "scratch_pool".
>...
> @@ -1133,21 +1200,25 @@ read_entries(svn_wc_adm_access_t *adm_ac
> SVN_ERR(fetch_base_nodes(&base_nodes, wc_db, scratch_pool, scratch_pool));
> SVN_ERR(fetch_working_nodes(&working_nodes, wc_db, scratch_pool,
> scratch_pool));
> + SVN_ERR(fetch_actual_nodes(&actual_nodes, wc_db, scratch_pool, scratch_pool));
>
> for (hi = apr_hash_first(scratch_pool, base_nodes); hi;
> hi = apr_hash_next(hi))
> {
> db_base_node_t *base_node;
> db_working_node_t *working_node;
> + db_actual_node_t *actual_node;
Let's see some "const" qualifiers on those node records!
>...
Cheers,
-g
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1154615
Re: svn commit: r35813 - branches/explore-wc/subversion/libsvn_wc
Posted by "Hyrum K. Wright" <hy...@mail.utexas.edu>.
On Feb 13, 2009, at 7:59 PM, Greg Stein wrote:
> On Wed, Feb 11, 2009 at 18:15, Hyrum K. Wright
> <hy...@hyrumwright.org> wrote:
>> ...
>> +++ branches/explore-wc/subversion/libsvn_wc/entries.c Wed Feb 11
>> 09:15:15 2009 (r35813)
>> ...
>> +/* Select all the rows from actual_node table in WC_DB and put
>> them into
>> + *NODES allocated in RESULT_POOL. */
>
> "all" ?!
>
> Please add a TODO about fixing that... we don't want to always read
> all rows. Our wc, client, and other code *does* tend to consume O(#
> committables), but in almost all other cases, it doesn't try to hold
> information about "all nodes" in a working copy.
There's a pretty sizable TODO in ready_entries() already. It doesn't
mention this specific issue, but it does say that there's a better way
to do things than we're currently doing them.
>> +static svn_error_t *
>> +fetch_actual_nodes(apr_hash_t **nodes,
>> + svn_sqlite__db_t *wc_db,
>> + apr_pool_t *scratch_pool,
>> + apr_pool_t *result_pool)
>> +{
>
> In wc_db.h and elsewhere, I've *always* declared these as
> "result_pool" first, then "scratch_pool".
Hmm, I'll add an internal todo, but I probably won't get to it for a
couple days.
>> ...
>> @@ -1133,21 +1200,25 @@ read_entries(svn_wc_adm_access_t *adm_ac
>> SVN_ERR(fetch_base_nodes(&base_nodes, wc_db, scratch_pool,
>> scratch_pool));
>> SVN_ERR(fetch_working_nodes(&working_nodes, wc_db, scratch_pool,
>> scratch_pool));
>> + SVN_ERR(fetch_actual_nodes(&actual_nodes, wc_db, scratch_pool,
>> scratch_pool));
>>
>> for (hi = apr_hash_first(scratch_pool, base_nodes); hi;
>> hi = apr_hash_next(hi))
>> {
>> db_base_node_t *base_node;
>> db_working_node_t *working_node;
>> + db_actual_node_t *actual_node;
>
> Let's see some "const" qualifiers on those node records!
r35872.
-Hyrum
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1155408