On Wed, Jan 26, 2011 at 7:15 PM, Johan Corveleyn <jcor...@gmail.com> wrote:
> On Wed, Jan 26, 2011 at 10:50 PM, Hyrum K Wright <hy...@hyrumwright.org>
> wrote:
>> On Tue, Jan 25, 2011 at 8:31 PM, Johan Corveleyn <jcor...@gmail.com> wrote:
>> ...
>>>> * revv svn_diff.h#svn_diff_fns_t []
>>>>
>>>> It looks like, for the most part, any destabilizing functionality is
>>>> completed, and what remains are simply optimizations. This
>>>> optimization work can probably be performed on trunk, and if so, we
>>>> should merge the branch back and do the cleanup work there. The only
>>>> bit on the current list of stuff is revving svn_diff_fns_t. Can that
>>>> work be parallelized?
>>>
>>> Yes, you are correct. Most of the essential work is done. Further
>>> optimizations can just as well be done on trunk.
>>>
>>> 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)?
>>
>> I was just wondering about how much could be done by other folks while
>> you were working on the primary goal of the branch. (While any full
>> committer can commit to the branch, it's generally good form not to
>> step on people's toes.)
>
> Oh yes, no problem. Any help with revv'ing that struct (or with other
> parts of the code) is very much appreciated. Feel free to make any
> changes necessary on the branch.
Johan,
I'd appreciate review on the attached patch. It is an attempt to rev
the svn_diff_fns_t struct and related functions. You'll notice that I
commented out the use of datasources_open in both diff_file.c and
diff_memory.c, in an attempt to exercise the deprecated function
wrappers.
It currently segfaults horribly, but I'm not completely sure why. I
suspect something is amiss in the implementation of the compat wrapper
for datasources_open in deprecated.c. If you'd review this patch, I'd
be much obliged.
Thanks,
-Hyrum
Index: subversion/libsvn_diff/deprecated.c
===================================================================
--- subversion/libsvn_diff/deprecated.c (revision 1064135)
+++ subversion/libsvn_diff/deprecated.c (working copy)
@@ -41,7 +41,61 @@
/*** Code. ***/
+struct fns_wrapper_baton
+{
+ /* We put the old baton in front of this one, so that we can still use
+ this baton in place of the old. This prevents us from having to
+ implement simple wrappers around each member of diff_fns_t. */
+ void *old_baton;
+ const svn_diff_fns_t *vtable;
+};
+static svn_error_t *
+datasources_open(void *baton, apr_off_t *prefix_lines,
+ svn_diff_datasource_e datasources[],
+ apr_size_t datasource_len)
+{
+ struct fns_wrapper_baton *fwb = baton;
+ int i;
+
+ /* Just iterate over the datasources, using the old singular version. */
+ for (i = 0; i < datasource_len; i++)
+ {
+ SVN_ERR(fwb->vtable->datasource_open(baton, datasources[i]));
+ }
+
+ /* Don't claim any prefix matches. */
+ *prefix_lines = 0;
+
+ return SVN_NO_ERROR;
+}
+
+static void
+wrap_diff_fns(svn_diff_fns2_t **diff_fns2,
+ struct fns_wrapper_baton **baton2,
+ const svn_diff_fns_t *diff_fns,
+ void *baton,
+ apr_pool_t *result_pool)
+{
+ /* Initialize the return vtable. */
+ *diff_fns2 = apr_palloc(result_pool, sizeof(**diff_fns2));
+
+ (*diff_fns2)->datasource_open = diff_fns->datasource_open;
+ (*diff_fns2)->datasource_close = diff_fns->datasource_close;
+ (*diff_fns2)->datasource_get_next_token =
diff_fns->datasource_get_next_token;
+ (*diff_fns2)->token_compare = diff_fns->token_compare;
+ (*diff_fns2)->token_discard = diff_fns->token_discard;
+ (*diff_fns2)->token_discard_all = diff_fns->token_discard_all;
+
+ (*diff_fns2)->datasources_open = datasources_open;
+
+ /* Initialize the wrapper baton. */
+ *baton2 = apr_palloc(result_pool, sizeof (**baton2));
+ (*baton2)->old_baton = baton;
+ (*baton2)->vtable = diff_fns;
+}
+
+
/*** From diff_file.c ***/
svn_error_t *
svn_diff_file_output_unified2(svn_stream_t *output_stream,
@@ -142,3 +196,48 @@ svn_diff_file_output_merge(svn_stream_t *output_st
style,
pool);
}
+
+
+/*** From diff.c ***/
+svn_error_t *
+svn_diff_diff(svn_diff_t **diff,
+ void *diff_baton,
+ const svn_diff_fns_t *vtable,
+ apr_pool_t *pool)
+{
+ svn_diff_fns2_t *diff_fns2;
+ struct fns_wrapper_baton *fwb;
+
+ wrap_diff_fns(&diff_fns2, &fwb, vtable, diff_baton, pool);
+ return svn_diff_diff_2(diff, fwb, diff_fns2, pool);
+}
+
+
+/*** From diff3.c ***/
+svn_error_t *
+svn_diff_diff3(svn_diff_t **diff,
+ void *diff_baton,
+ const svn_diff_fns_t *vtable,
+ apr_pool_t *pool)
+{
+ svn_diff_fns2_t *diff_fns2;
+ struct fns_wrapper_baton *fwb;
+
+ wrap_diff_fns(&diff_fns2, &fwb, vtable, diff_baton, pool);
+ return svn_diff_diff3_2(diff, fwb, diff_fns2, pool);
+}
+
+
+/*** From diff4.c ***/
+svn_error_t *
+svn_diff_diff4(svn_diff_t **diff,
+ void *diff_baton,
+ const svn_diff_fns_t *vtable,
+ apr_pool_t *pool)
+{
+ svn_diff_fns2_t *diff_fns2;
+ struct fns_wrapper_baton *fwb;
+
+ wrap_diff_fns(&diff_fns2, &fwb, vtable, diff_baton, pool);
+ return svn_diff_diff4_2(diff, fwb, diff_fns2, pool);
+}
Index: subversion/libsvn_diff/diff_memory.c
===================================================================
--- subversion/libsvn_diff/diff_memory.c (revision 1064135)
+++ subversion/libsvn_diff/diff_memory.c (working copy)
@@ -202,7 +202,7 @@ token_discard_all(void *baton)
static const svn_diff_fns_t svn_diff__mem_vtable =
{
datasource_open,
- datasources_open,
+ /*datasources_open,*/
datasource_close,
datasource_get_next_token,
token_compare,
Index: subversion/libsvn_diff/token.c
===================================================================
--- subversion/libsvn_diff/token.c (revision 1064135)
+++ subversion/libsvn_diff/token.c (working copy)
@@ -137,7 +137,7 @@ svn_error_t *
svn_diff__get_tokens(svn_diff__position_t **position_list,
svn_diff__tree_t *tree,
void *diff_baton,
- const svn_diff_fns_t *vtable,
+ const svn_diff_fns2_t *vtable,
svn_diff_datasource_e datasource,
apr_off_t prefix_lines,
apr_pool_t *pool)
Index: subversion/libsvn_diff/diff_file.c
===================================================================
--- subversion/libsvn_diff/diff_file.c (revision 1064135)
+++ subversion/libsvn_diff/diff_file.c (working copy)
@@ -1064,7 +1064,7 @@ token_discard_all(void *baton)
static const svn_diff_fns_t svn_diff__file_vtable =
{
datasource_open,
- datasources_open,
+ /*datasources_open,*/
datasource_close,
datasource_get_next_token,
token_compare,
Index: subversion/libsvn_diff/diff.c
===================================================================
--- subversion/libsvn_diff/diff.c (revision 1064135)
+++ subversion/libsvn_diff/diff.c (working copy)
@@ -98,10 +98,10 @@ svn_diff__diff(svn_diff__lcs_t *lcs,
svn_error_t *
-svn_diff_diff(svn_diff_t **diff,
- void *diff_baton,
- const svn_diff_fns_t *vtable,
- apr_pool_t *pool)
+svn_diff_diff_2(svn_diff_t **diff,
+ void *diff_baton,
+ const svn_diff_fns2_t *vtable,
+ apr_pool_t *pool)
{
svn_diff__tree_t *tree;
svn_diff__position_t *position_list[2];
Index: subversion/libsvn_diff/diff3.c
===================================================================
--- subversion/libsvn_diff/diff3.c (revision 1064135)
+++ subversion/libsvn_diff/diff3.c (working copy)
@@ -244,10 +244,10 @@ svn_diff__resolve_conflict(svn_diff_t *hunk,
svn_error_t *
-svn_diff_diff3(svn_diff_t **diff,
- void *diff_baton,
- const svn_diff_fns_t *vtable,
- apr_pool_t *pool)
+svn_diff_diff3_2(svn_diff_t **diff,
+ void *diff_baton,
+ const svn_diff_fns2_t *vtable,
+ apr_pool_t *pool)
{
svn_diff__tree_t *tree;
svn_diff__position_t *position_list[3];
Index: subversion/libsvn_diff/diff4.c
===================================================================
--- subversion/libsvn_diff/diff4.c (revision 1064135)
+++ subversion/libsvn_diff/diff4.c (working copy)
@@ -166,10 +166,10 @@ adjust_diff(svn_diff_t *diff, svn_diff_t *adjust)
}
svn_error_t *
-svn_diff_diff4(svn_diff_t **diff,
- void *diff_baton,
- const svn_diff_fns_t *vtable,
- apr_pool_t *pool)
+svn_diff_diff4_2(svn_diff_t **diff,
+ void *diff_baton,
+ const svn_diff_fns2_t *vtable,
+ apr_pool_t *pool)
{
svn_diff__tree_t *tree;
svn_diff__position_t *position_list[4];
Index: subversion/libsvn_diff/diff.h
===================================================================
--- subversion/libsvn_diff/diff.h (revision 1064135)
+++ subversion/libsvn_diff/diff.h (working copy)
@@ -110,7 +110,7 @@ svn_error_t *
svn_diff__get_tokens(svn_diff__position_t **position_list,
svn_diff__tree_t *tree,
void *diff_baton,
- const svn_diff_fns_t *vtable,
+ const svn_diff_fns2_t *vtable,
svn_diff_datasource_e datasource,
apr_off_t prefix_lines,
apr_pool_t *pool);
Index: subversion/include/svn_diff.h
===================================================================
--- subversion/include/svn_diff.h (revision 1064135)
+++ subversion/include/svn_diff.h (working copy)
@@ -106,7 +106,7 @@ typedef enum svn_diff_datasource_e
/** A vtable for reading data from the three datasources. */
-typedef struct svn_diff_fns_t
+typedef struct svn_diff_fns2_t
{
/** Open the datasource of type @a datasource. */
svn_error_t *(*datasource_open)(void *diff_baton,
@@ -146,6 +146,48 @@ typedef enum svn_diff_datasource_e
/** Free *all* tokens from memory, they're no longer needed. */
void (*token_discard_all)(void *diff_baton);
+} svn_diff_fns2_t;
+
+
+/** A vtable for reading data from the three datasources.
+ *
+ * @deprecated Provided for backward compatibility with the 1.6 API.
+ */
+typedef struct svn_diff_fns_t
+{
+ /** Open the datasource of type @a datasource. */
+ svn_error_t *(*datasource_open)(void *diff_baton,
+ svn_diff_datasource_e datasource);
+
+ /** Close the datasource of type @a datasource. */
+ svn_error_t *(*datasource_close)(void *diff_baton,
+ svn_diff_datasource_e datasource);
+
+ /** Get the next "token" from the datasource of type @a datasource.
+ * Return a "token" in @a *token. Return a hash of "token" in @a *hash.
+ * Leave @a token and @a hash untouched when the datasource is exhausted.
+ */
+ svn_error_t *(*datasource_get_next_token)(apr_uint32_t *hash, void **token,
+ void *diff_baton,
+ svn_diff_datasource_e datasource);
+
+ /** A function for ordering the tokens, resembling 'strcmp' in functionality.
+ * @a compare should contain the return value of the comparison:
+ * If @a ltoken and @a rtoken are "equal", return 0. If @a ltoken is
+ * "less than" @a rtoken, return a number < 0. If @a ltoken is
+ * "greater than" @a rtoken, return a number > 0.
+ */
+ svn_error_t *(*token_compare)(void *diff_baton,
+ void *ltoken,
+ void *rtoken,
+ int *compare);
+
+ /** Free @a token from memory, the diff algorithm is done with it. */
+ void (*token_discard)(void *diff_baton,
+ void *token);
+
+ /** Free *all* tokens from memory, they're no longer needed. */
+ void (*token_discard_all)(void *diff_baton);
} svn_diff_fns_t;
@@ -154,8 +196,23 @@ typedef enum svn_diff_datasource_e
/** Given a vtable of @a diff_fns/@a diff_baton for reading datasources,
* return a diff object in @a *diff that represents a difference between
* an "original" and "modified" datasource. Do all allocation in @a pool.
+ *
+ * @since New in 1.7.
*/
svn_error_t *
+svn_diff_diff_2(svn_diff_t **diff,
+ void *diff_baton,
+ const svn_diff_fns2_t *diff_fns,
+ apr_pool_t *pool);
+
+/** Given a vtable of @a diff_fns/@a diff_baton for reading datasources,
+ * return a diff object in @a *diff that represents a difference between
+ * an "original" and "modified" datasource. Do all allocation in @a pool.
+ *
+ * @deprecated Provided for backward compatibility with the 1.6 API.
+ */
+SVN_DEPRECATED
+svn_error_t *
svn_diff_diff(svn_diff_t **diff,
void *diff_baton,
const svn_diff_fns_t *diff_fns,
@@ -165,8 +222,24 @@ svn_diff_diff(svn_diff_t **diff,
* return a diff object in @a *diff that represents a difference between
* three datasources: "original", "modified", and "latest". Do all
* allocation in @a pool.
+ *
+ * @since New in 1.7.
*/
svn_error_t *
+svn_diff_diff3_2(svn_diff_t **diff,
+ void *diff_baton,
+ const svn_diff_fns2_t *diff_fns,
+ apr_pool_t *pool);
+
+/** Given a vtable of @a diff_fns/@a diff_baton for reading datasources,
+ * return a diff object in @a *diff that represents a difference between
+ * three datasources: "original", "modified", and "latest". Do all
+ * allocation in @a pool.
+ *
+ * @deprecated Provided for backward compatibility with the 1.6 API.
+ */
+SVN_DEPRECATED
+svn_error_t *
svn_diff_diff3(svn_diff_t **diff,
void *diff_baton,
const svn_diff_fns_t *diff_fns,
@@ -177,8 +250,25 @@ svn_diff_diff3(svn_diff_t **diff,
* two datasources: "original" and "latest", adjusted to become a full
* difference between "original", "modified" and "latest" using "ancestor".
* Do all allocation in @a pool.
+ *
+ * @since New in 1.7.
*/
svn_error_t *
+svn_diff_diff4_2(svn_diff_t **diff,
+ void *diff_baton,
+ const svn_diff_fns2_t *diff_fns,
+ apr_pool_t *pool);
+
+/** Given a vtable of @a diff_fns/@a diff_baton for reading datasources,
+ * return a diff object in @a *diff that represents a difference between
+ * two datasources: "original" and "latest", adjusted to become a full
+ * difference between "original", "modified" and "latest" using "ancestor".
+ * Do all allocation in @a pool.
+ *
+ * @deprecated Provided for backward compatibility with the 1.6 API.
+ */
+SVN_DEPRECATED
+svn_error_t *
svn_diff_diff4(svn_diff_t **diff,
void *diff_baton,
const svn_diff_fns_t *diff_fns,
