Eliminating multiple file-reads of text-base on updated targets

[Erik Huelsmann <eh...@gmail.com>]

I've found an optimization which would fit nicely in the 1.4
'libsvn_wc optimizing release': libsvn_wc calls svn_io_file_checksum
on files it just read or wrote - meaning it reads them again to
calculate the checksum.

My plan involves:

- creating an svn_stream_checksummed, like svn_subst_stream_translated.
- let the stream raise an error on close if the stream checksum
doesn't match a given checksum value (with an error message text
passed to it on creation)
- create svn_stream_checksummed_get_sum() to retrieve the checksum
from the checksummed stream
- add the checksumming stream to the source stream in
adm_crawler.c:svn_wc_transmit_text_deltas()

The last step optimizes the common case which is supposed to pass the
checksum test. In the uncommon case of not passing the checksum test,
we read too much off the network, but the common case reads the text
base twice.


The svn_stream_checksummed_get_sum() can be used later to do something
similar with streams that are being written, for which we need the
checksum later on.

Comments?


bye,


Erik.

Re: Eliminating multiple file-reads of text-base on updated targets

[Erik Huelsmann <eh...@gmail.com>]

On 12/6/05, Peter N. Lundblad <pe...@famlundblad.se> wrote:
> On Tue, 6 Dec 2005, Erik Huelsmann wrote:
>
> > I've found an optimization which would fit nicely in the 1.4
> >'libsvn_wc optimizing release': libsvn_wc calls svn_io_file_checksum
> >on files it just read or wrote - meaning it reads them again to
> >calculate the checksum.
>
> Do you know how much this costs performance-wise? Yes, it might be a
> reasonable optimization, but it would be good to have the whole picture if
> possible.

Absolutely no idea, but I guess it makes a big difference for what
your working copy looks like: small and medium size files will
probably just hit the penalty of accessing the filesystem, but large
(hundreds of MBs) files will suffer big time if they need to be read
several times.

> >- creating an svn_stream_checksummed, like svn_subst_stream_translated.
> >- let the stream raise an error on close if the stream checksum
> >doesn't match a given checksum value (with an error message text
> >passed to it on creation)
>
> Would it be better to just let the stream provide the checksum and let the
> caller take care of the error handling itself?  I see this passing of
> error messages down the calls as a little ugly.

Yes, I was posting about it exactly because of it.

> >- create svn_stream_checksummed_get_sum() to retrieve the checksum
> >from the checksummed stream
>
> Just provide a pointer where to store the checksum on close instead? The
> function above wouldn't be type-safe, would it?

Storing in a pointer passed to it will also work. Heh. Thanks for the
idea. Never thought about it.

> BTW, this is the big part of issue #1882.

Thanks for finding the issue :-) I knew there was an issue about
something like it, but it's only a related problem in this case: it's
the calculation of the target datastream which is meant in the issue. 
My problem is that we calculate the MD5 of the target stream, write
the target stream to disc, forget the MD5 and need to recalculate the
MD5 later on... (But that's part 2. What I presented here is part 1.)

Part 2 involves update_editor.c.

bye,


Erik.

Re: Eliminating multiple file-reads of text-base on updated targets

["Peter N. Lundblad" <pe...@famlundblad.se>]

On Tue, 6 Dec 2005, Erik Huelsmann wrote:

> I've found an optimization which would fit nicely in the 1.4
>'libsvn_wc optimizing release': libsvn_wc calls svn_io_file_checksum
>on files it just read or wrote - meaning it reads them again to
>calculate the checksum.

Do you know how much this costs performance-wise? Yes, it might be a
reasonable optimization, but it would be good to have the whole picture if
possible.

>- creating an svn_stream_checksummed, like svn_subst_stream_translated.
>- let the stream raise an error on close if the stream checksum
>doesn't match a given checksum value (with an error message text
>passed to it on creation)

Would it be better to just let the stream provide the checksum and let the
caller take care of the error handling itself?  I see this passing of
error messages down the calls as a little ugly.


>- create svn_stream_checksummed_get_sum() to retrieve the checksum
>from the checksummed stream

Just provide a pointer where to store the checksum on close instead? The
function above wouldn't be type-safe, would it?


BTW, this is the big part of issue #1882.

Regards,
//Peter

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org