Charles Acknin writes:
> Daniel and I had a little talk on #svn-dev about something that seemed
> a bit weird in libsvn_wc/diff.c I noticed as running within gdb while
> writing my GSoC proposal: for single-file diff on WC/WC (e.g. svn
> diff foo), directory_elements_diff() would keep looping (line 692, rev
> 24199) until all files of foo's directory have been processed instead
> of breaking when foo is done. Those few lines will ensure we 'break'
> when files passed as argument are processed.
>
If I understand what you're saying correctly, this is not a correctness
bug, but just a minor performance glitch?
> [[[
> Prevent from some useless processing when performing single-file diff on WC/WC.
>
Seems like that;)
> * subversion/libsvn_wc/diff.c
> (directory_elements_diff): add a boolean so that we know when the
> file passed as argument is diffed
> ]]]
>
>
>
> --- subversion/libsvn_wc/diff.c (revision 24199)
> +++ subversion/libsvn_wc/diff.c (working copy)
> @@ -697,6 +697,7 @@
> const svn_wc_entry_t *entry;
> struct dir_baton *subdir_baton;
> const char *name, *path;
> + svn_boolean_t file_diffed = FALSE;
>
> svn_pool_clear(subpool);
>
> @@ -726,6 +727,7 @@
> {
> case svn_node_file:
> SVN_ERR(file_diff(dir_baton, path, entry, subpool));
> + file_diffed = TRUE;
> break;
>
It looks like this *always* breaks out of the loop after the first file.
Is this correct when the WC have more than one changed file/directory?
> case svn_node_dir:
> @@ -756,6 +758,9 @@
> default:
> break;
> }
> +
> + if (file_diffed)
> + break;
> }
>
> svn_pool_destroy(subpool);
>
I guess the useless work you're after happens because the anchor and target
are different and we're still looping over all entries in the anchor
directory. If you think that's a problem, maybe we should just get the entry
we want in this case. If you can find a way to do that without extra code
duplication added, then it might be worth it, but I doubt you can measure
any performance noticable difference in a reasonably sized directory.
Hint: Running "make check" before submitting a patch helps make sure it
doesn't break anything;)
Best,
//Peter
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Mar 28 13:28:17 2007