On Wed, Jan 23, 2013 at 9:39 PM, Paul Burba <ptbu...@gmail.com> wrote: > On Wed, Jan 23, 2013 at 9:51 AM, Julian Foad <julianf...@btopenworld.com> > wrote: >> Ivan Zhakov wrote: >> >>> I was testing recent changes in ra_serf update editor and noticed that >>> reintegrate-like merges for long living branches are extremely slow. >>> Client requests server for diff between branches with respect to >>> ancestry and servers reports no-op txdelta for every file that was >>> changed in original branch. Then for every such file client retrieves >>> text and properties. >>> >>> For example try to reintegrate ev2-export branch back to trunk. [...]
Hi Bert and Paul! On Wed, Jan 23, 2013 at 6:51 PM, Bert Huijben <b...@qqmail.nl> wrote: > Is there a way the diff code can see that it receives a no-op txdelta? > > In that case it can just skip retrieving the file as it won't be able to > produce a difference anyway. > It would be really hard to implement on client, because no-op txdelta transmitted as full txdelta with instructions to copy all source data. But we can detect such no-op txdelta on server side. See my plan bellow. >> Thanks Ivan, that's very interesting. I'll take a look, since I have just >> been working on that code. >> >> I exposed this as a separate option because the > two meanings of the previous 'ignore_ancestry' option were conflated, but I > don't deeply understand what happens when this option is specified. I know > what it's > supposed to mean at a basic level: "diff a pair of unrelated nodes as if they > are related". What I don't know is how well it's implemented and whether > it's really useful when merging. >> >>> What is the purpose of diff_ignore_ancestry for merges? Can we default >>> it to FALSE? >> >> I assume one of the purposes is if your work flow has been such > that a file is sometimes replaced (without copy-from) > > Any replacement is a problem, regardless of copy-from. For example: > [...] > >>, and sometimes a new file added on one branch has been added on the > other branch by a simple add (without copy-from). In that sort of work flow, > the 'diff_ignore_ancestry' would cause Subversion to do diffs between > different versions of a file, and that might well help in merging the changes. > Without the 'diff_ignore_ancestry' option, Subversion would treat the file as > 'replaced', and so you'd be likely to get a tree conflict when you try to > merge it. > Thanks for demonstrating use-case. That what I was looking for. We cannot change to ignore_ancestry = TRUE by default, but I'm going to try the following: 1. Add test case for replacement merge that you demonstrated (it would be useful anyway) 2. Modify delta reporter on server side to do not send deltas if files have the same content, regardless ancestry (see subversion/libsvn_repos/reporter.c:696 delta_files()) Patch attached. 3. Test it and think a little bit more is it safe change or not :) -- Ivan Zhakov
Index: subversion/libsvn_repos/reporter.c =================================================================== --- subversion/libsvn_repos/reporter.c (revision 1441178) +++ subversion/libsvn_repos/reporter.c (working copy) @@ -693,12 +693,9 @@ changed with respect to" and "has the same actual contents as". We'll do everything we can to avoid transmitting even an empty text-delta in that case. */ - if (b->ignore_ancestry) - SVN_ERR(svn_repos__compare_files(&changed, b->t_root, t_path, - s_root, s_path, pool)); - else - SVN_ERR(svn_fs_contents_changed(&changed, b->t_root, t_path, s_root, - s_path, pool)); + SVN_ERR(svn_repos__compare_files(&changed, b->t_root, t_path, + s_root, s_path, pool)); + if (!changed) return SVN_NO_ERROR;