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 <[email protected]> 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; > } > >
