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

Re: [PATCH] Elminate duplicate MD5 calculation in FSFS

From: Greg Stein <gstein_at_gmail.com>
Date: Tue, 6 Apr 2010 14:51:59 -0400

When I wrote svn_txdelta_run() a while back, I seem to recall that it
could be used in the FS library, if a couple function calls get
collapsed. i.e part of the process is started in one function, then
completed in another function. But overall, those two pieces do what
svn_txdelta_run() does. And... txdelta_run allows for skipping the
checksum thing.

Is this code amenable to using svn_txdelta_run? That would simplify
things, and we wouldn't need another entry point.

Cheers,
-g

On Tue, Apr 6, 2010 at 14:24, Stefan Fuhrman
<stefanfuhrmann_at_alice-dsl.de> wrote:
> Hi devs,
>
> FSFS calculates and checks MD5 in rep_read_contents.
> So, there is no reason so calculate it a second time in the
> delta stream svn_fs_fs__get_file_delta_stream returns.
> However, it gets calculated there "accidentally" and not
> used later.
>
> This patch adds a variant of svn_txdelta() that will not
> calculate the checksum. It uses that new function in FSFS.
>
> -- Stefan^2.
>
> [[[
> Eliminate superfluous checksum calculation when
> reading deltas from FSFS.
>
> * subversion/include/svn_delta.h
>  (svn_txdelta_unchecked): declare new function
>
> * subversion/libsvn_delta/text_delta.c
>  add include for the _() nls macro
>  (txdelta_baton): extend commentary
>  (txdelta_no_digest, svn_txdelta_unchecked):
>  new functions
>  (svn_txdelta_send_stream): make sure to detect
>  cases where svn_txdelta_unchecked was used
>  but not appropriate
>
> * subversion/libsvn_fs_fs/fs_fs.c
>  (svn_fs_fs__get_file_delta_stream): return
>  a non-checksumming stream.
>
> patch by stefanfuhrmann < at > alice-dsl.de
> ]]]
>
> Index: subversion/include/svn_delta.h
> ===================================================================
> --- subversion/include/svn_delta.h      (revision 930220)
> +++ subversion/include/svn_delta.h      (working copy)
> @@ -353,7 +353,22 @@
>             svn_stream_t *target,
>             apr_pool_t *pool);
>
> +/** Just like @ref svn_txdelta except that no checksum is being created.
> + * When calling @ref svn_txdelta_md5_digest with @a *stream, it will
> + * return @c NULL.
> + *
> + * Use this function only if the @a source stream comes from a trustworthy
> + * source as e.g. an in-process FS driver.
> + *
> + * @since New in 1.7.
> + */
> +void
> +svn_txdelta_unchecked(svn_txdelta_stream_t **stream,
> +                      svn_stream_t *source,
> +                      svn_stream_t *target,
> +                      apr_pool_t *pool);
>
> +
>  /**
>  * Return a writable stream which, when fed target data, will send
>  * delta windows to @a handler/@a handler_baton which transform the
> Index: subversion/libsvn_delta/text_delta.c
> ===================================================================
> --- subversion/libsvn_delta/text_delta.c        (revision 930220)
> +++ subversion/libsvn_delta/text_delta.c        (working copy)
> @@ -32,6 +32,7 @@
>  #include "svn_io.h"
>  #include "svn_pools.h"
>  #include "svn_checksum.h"
> +#include "svn_private_config.h"
>
>  #include "delta.h"
>
> @@ -57,7 +58,7 @@
>   svn_filesize_t pos;           /* Offset of next read in source file. */
>   char *buf;                    /* Buffer for input data. */
>
> -  svn_checksum_ctx_t *context;  /* Context for computing the checksum. */
> +  svn_checksum_ctx_t *context;  /* Context for computing the checksum. May
> be NULL. */
>   svn_checksum_t *checksum;     /* If non-NULL, the checksum of TARGET. */
>
>   apr_pool_t *result_pool;      /* For results (e.g. checksum) */
> @@ -368,6 +369,13 @@
>  }
>
>
> +static const unsigned char *
> +txdelta_no_digest(void *baton)
> +{
> +  return NULL;
> +}
> +
> +
>  svn_error_t *
>  svn_txdelta_run(svn_stream_t *source,
>                 svn_stream_t *target,
> @@ -440,7 +448,26 @@
>                                       txdelta_md5_digest, pool);
>  }
>
> +void
> +svn_txdelta_unchecked(svn_txdelta_stream_t **stream,
> +                      svn_stream_t *source,
> +                      svn_stream_t *target,
> +                      apr_pool_t *pool)
> +{
> +  struct txdelta_baton *b = apr_pcalloc(pool, sizeof(*b));
>
> +  b->source = source;
> +  b->target = target;
> +  b->more_source = TRUE;
> +  b->more = TRUE;
> +  b->buf = apr_palloc(pool, 2 * SVN_DELTA_WINDOW_SIZE);
> +  b->result_pool = pool;
> +
> +  *stream = svn_txdelta_stream_create(b, txdelta_next_window,
> +                                      txdelta_no_digest, pool);
> +}
> +
> +
>
>  /* Functions for implementing a "target push" delta. */
>
>
> @@ -814,7 +859,14 @@
>     {
>       const unsigned char *result_md5;
>       result_md5 = svn_txdelta_md5_digest(txstream);
> -      /* Since err is null, result_md5 "cannot" be null. */
> +
> +      /* Don't call this function for streams created with
> +         svn_txdelta_unchecked. Or txdelta_md5_digest was
> +         called prematurely. */
> +      if (result_md5 == NULL)
> +        return svn_error_create(SVN_ERR_INCORRECT_PARAMS, NULL,
> +                                _("Unchecked or incomplete stream"));
> +
>       memcpy(digest, result_md5, APR_MD5_DIGESTSIZE);
>     }
>
>   return err;
> Index: subversion/libsvn_fs_fs/fs_fs.c
> ===================================================================
> --- subversion/libsvn_fs_fs/fs_fs.c     (revision 930220)
> +++ subversion/libsvn_fs_fs/fs_fs.c     (working copy)
> @@ -3481,13 +3481,14 @@
>         SVN_ERR(svn_io_file_close(rep_state->file, pool));
>     }
>
> -  /* Read both fulltexts and construct a delta. */
> +  /* Read both fulltexts and construct a delta. The checksum for stream_p
> +     will not be used by the callers. Thus, don't calculate it. */
>   if (source)
>     SVN_ERR(read_representation(&source_stream, fs, source->data_rep,
> pool));
>   else
>     source_stream = svn_stream_empty(pool);
>   SVN_ERR(read_representation(&target_stream, fs, target->data_rep, pool));
> -  svn_txdelta(stream_p, source_stream, target_stream, pool);
> +  svn_txdelta_unchecked(stream_p, source_stream, target_stream, pool);
>
>   return SVN_NO_ERROR;
>  }
>
>
Received on 2010-04-06 20:52:27 CEST

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.