[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: diff --summarize

From: Julian Foad <julian.foad_at_wandisco.com>
Date: Fri, 26 Aug 2011 16:47:13 +0100

On Thu, 2011-08-25, I (Julian Foad) wrote:
> I discovered today that the network traffic generated by my rewrite of
> "diff --summarize" is ridiculously heavy - like apparently 100 times
> what it was, in a simple real-world test. I have an obvious patch which
> I'll apply soon which eliminates the requests for content of deleted
> files, and that reduces it by a factor of ten

Committed in r1162040.

> , but I haven't yet figured
> out where the other 10x is happening. I'll figure it out or else I'll
> have to revert it all, of course.

I figured out it's basically because of collecting info about prop
changes. The editor we're using now in repos_diff.c collects and
reports prop changes in full [1] through the diff callbacks. That
involves requesting the "pristine" (or "left-hand side") props, which
often requires another network round-trip. Previously, the dedicated
summarize editor just accepted any prop change reported by the
repository as being evidence enough.

The obvious fix is to make the client diff editory (repos_diff.c) not
fetch the original props when in summary mode, but that's not
necessarily the right thing to do.

I think there's a correctness issue here, related to a quirk described
in issue #3657 "dav update report handler in skelta mode can cause
spurious conflicts", which is summarized in the doc string of
repos_diff.c:remove_non_prop_changes(). Basically, sometimes the RA
layer will send a no-op prop "change" which is not actually a change.
If we want to know whether a reported prop "change" is actually a
change, we have to fetch the original props and compare.

The previous implementation of --summarize must have been reporting a
prop change if and only if the RA layer said so through the delta
editor. I think that must have been wrong. I'll try to come up with a
test case.

Note that there is also a separate complication in reporting prop
changes, which is that WC-props and entry-props also get reported but
are not relevant so we have to ignore them. That's unrelated to what
I'm talking about here.

[1] Prop changes are reported "in full" only to the extent that the RA
layer sends them. I discovered that RA-neon doesn't send the full prop
changes if it knows it's omitting text deltas. That's in
libsvn_ra_neon/fetch.c:start_element(), case ELEM_fetch_props, where
'fetch_content' is the 'text_deltas' parameter of
svn_ra_neon__do_diff().

- Julian
Received on 2011-08-26 17:47:51 CEST

This is an archived mail posted to the Subversion Dev mailing list.