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

Re: Status of the branch diff-optimizations-bytes branch

From: Johan Corveleyn <jcorvel_at_gmail.com>
Date: Wed, 26 Jan 2011 15:18:24 +0100

On Wed, Jan 26, 2011 at 2:46 PM, Hyrum K Wright <hyrum_at_hyrumwright.org> wrote:
> On Tue, Jan 25, 2011 at 11:41 PM, Daniel Shahaf <d.s_at_daniel.shahaf.name> wrote:
>> Johan Corveleyn wrote on Wed, Jan 26, 2011 at 03:31:11 +0100:
>>> Revving svn_diff_fns_t: what do you mean with parallelizing it? I must
>>> admit that I don't really know (yet) how to go about that. Very early
>>> during the branch work, danielsh pointed out that I modified this
>>> public struct (vtable for reading data from datasources), so it should
>>> be revved. Is it listed/documented somewhere what should be done for
>>> that (community guide perhaps)?
>>
>> Briefly, revving a function includes re-declaring it, updating the old
>> declaration to be a diff against the new one, marking it as deprecated
>> (using doxygen and C preprocessor designators), and re-implementing the
>> old function as a deprecated.c wrapper around the new one.
>>
>> For a struct, you need to re-declare the struct and revv functions that
>> use it.  Also, don't forget to add a constructor function
>> (svn_foo_t_create(), but s/_t_/_/) (and possibly a duplicator function),
>> and forbid people from defining variables of the struct type directly.
>>
>> I'm not sure HACKING contains this.  On the other hand, the public
>> header files contain plenty of examples of everything I just said...
>
> For testing purposes, I usually find it easiest to work "from the
> bottom up".  In other words, rev the lowest API in the call stack,
> write the appropriate wrapper for the existing API, and test and
> commit.  This ensures that the existing API (now implemented as a
> wrapper) still behaves the same way, but it doesn't yet exercise
> whatever the new functionality is.  I then just work my way up the
> stack for each related function.  Structs are a little more involved,
> but can use similar principles.
>
> Johan, if you don't mind, I can put some of my (admittedly limited)
> tuits into looking at what needs to be done to rev the above struct.

I would definitely, absolutely not mind at all :-). Quite the contrary.

As a summary of what I did with svn_diff_fns_t (all in the context of
subversion/libsvn_diff):

- Added new function 'datasources_open' to svn_diff_fns_t (maybe
better to be named 'datasources_open_all' or something to have more
than 1 character difference with the other existing function
'datasource_open' :-), but I said that already).

- This new function is implemented in diff_file.c (the 'real' work of
prefix/suffix scanning) and in diff_memory.c (dummy implementation -
doesn't do anything for prefix/suffix scanning). Those were the only
internal implementors/providers of that vtable.

- I sent a mail once to the dev-list, asking if there were other known
implementors [1], but got no response. Of course, that's not a
definitive survey :-).

- The new vtable->datasources_open function is called from diff.c,
diff3.c and diff4.c.

- The only internal caller of the "old" function 'datasource_open'
(for a single datasource) doesn't call it anymore
(token.c#svn_diff__get_tokens) (there is no need anymore, since the
callers in diff.c, diff3.c and diff4.c already open the datasources
with 'datasources_open' before calling svn_diff__get_tokens).

Cheers,

-- 
Johan
[1] http://svn.haxx.se/dev/archive-2010-12/0083.shtml
Received on 2011-01-26 15:19:25 CET

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