You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Kamesh Jayachandran <ka...@collab.net> on 2006/10/10 10:08:29 UTC
[PATCH][merge-tracking] remove redundant for loop in svn_fs_merge_info__get_merge_info
Hi All,
Find the attached patch and log.
With regards
Kamesh Jayachandran
Re: [PATCH][merge-tracking] remove redundant for loop in svn_fs_merge_info__get_merge_info
Posted by Kamesh Jayachandran <ka...@collab.net>.
Daniel Berlin wrote:
>> Index: subversion/libsvn_fs_util/merge-info-sqlite-index.c
>> ===================================================================
>> --- subversion/libsvn_fs_util/merge-info-sqlite-index.c (revision 21862)
>> +++ subversion/libsvn_fs_util/merge-info-sqlite-index.c (working copy)
>> @@ -519,18 +519,12 @@
>> for (i = 0; i < paths->nelts; i++)
>> {
>> const char *path = APR_ARRAY_IDX(paths, i, const char *);
>> + apr_hash_t *currhash;
>> + svn_stringbuf_t *mergestring;
>>
>> SVN_ERR (get_merge_info_for_path (db, path, rev, *mergeinfo,
>> mergeinfo_cache, TRUE,
>> include_parents, pool));
>> - }
>> -
>
> Uh, changing this like you have would be a bug waiting to happen.
> There is no guarantee that *mergeinfo isn't going to get info about
> path from a *later* call to get_merge_info_for_path, so it's not
> enough to just lookup the info immediately after the call for *that*
> path.
You mean later call's can overwrite the mergeinfo hash for the same key?
Two things,
1)'get_merge_info_for_path' can touch 'mergeinfo' only for the given
path not for any other parent path.
2)if the 'paths' array has duplicate 'path' 'get_merge_info_for_path'
would return some info which should be same or atleast the latest
info(due to fresh commits) which should be fine to consume immedeately,
instead of one more for loop.
If you consider the mergeinfo alzebra of converting apr_hash_to to
'svn_merge_rangelist'... is costlier may be we can change the signature
of paths from 'apr_array_header_t *' to 'apr_hash_t*'.
With regards
Kamesh Jayachandran
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH][merge-tracking] remove redundant for loop in svn_fs_merge_info__get_merge_info
Posted by Daniel Berlin <db...@dberlin.org>.
> Index: subversion/libsvn_fs_util/merge-info-sqlite-index.c
> ===================================================================
> --- subversion/libsvn_fs_util/merge-info-sqlite-index.c (revision 21862)
> +++ subversion/libsvn_fs_util/merge-info-sqlite-index.c (working copy)
> @@ -519,18 +519,12 @@
> for (i = 0; i < paths->nelts; i++)
> {
> const char *path = APR_ARRAY_IDX(paths, i, const char *);
> + apr_hash_t *currhash;
> + svn_stringbuf_t *mergestring;
>
> SVN_ERR (get_merge_info_for_path (db, path, rev, *mergeinfo,
> mergeinfo_cache, TRUE,
> include_parents, pool));
> - }
> -
Uh, changing this like you have would be a bug waiting to happen.
There is no guarantee that *mergeinfo isn't going to get info about
path from a *later* call to get_merge_info_for_path, so it's not
enough to just lookup the info immediately after the call for *that*
path.
That is why they are two loops, one to get all the info, and then one
to process it.
> - for (i = 0; i < paths->nelts; i++)
> - {
> - svn_stringbuf_t *mergestring;
> - apr_hash_t *currhash;
> - const char *path = APR_ARRAY_IDX(paths, i, const char *);
> -
> currhash = apr_hash_get(*mergeinfo, path, APR_HASH_KEY_STRING);
> if (currhash)
> {
> @@ -539,6 +533,7 @@
> apr_hash_set(*mergeinfo, path, APR_HASH_KEY_STRING, mergestring->data);
> }
> }
> +
> SQLITE_ERR(sqlite3_close(db), db);
> return SVN_NO_ERROR;
> }
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org