Peter N. Lundblad wrote:
> Sebastian Tusk writes:
> > I did some tests with a repository that contains a single file sized 686
> > MB. I used the svn client version 1.3.0 on a windows machine.
> >
> [...]
> > svn commit
> > The file to commit is named test. The function performed is guessed.
> >
> > activity function? time
> > ------
> > read 512b blocks from "test"
> > write 512b blocks to "test.svn_base.tmp" COPY? 3m10s
>
> Copy the file to a temporary text base. 512 byte blocks are used on
> Windows because APRs file copying routine uses BUFSIZ which is far too
> small on Windows.
Ah, yes. In fact, APR doesn't even have a Windows-specific
implementation of apr_file_copy (which should use the hopefully optimal
CopyFile API).
> See
> <http://svn.haxx.se/dev/archive-2004-11/0123.shtml> for some
> discussion. I think we should have our own copy function (as a
> short-term solution) if it improves performance dramatically on some platforms.
>
Probably most, not just some platforms. Of course it depends on the
filesystem and other considerations, but BUFSIZ is intended for use by
the stdio library, not raw file IO.
> There have been some work in this area for 1.4. For example, temporary
> file creation for MD5 hashing has been eliminated in some cases. It
> would be nice to see results using a trunk build. And it would be nice
> to see the performance changes when increasing BUFSIZ. (Oh, and it
> seems that svn_io_file_checksum also uses BUFGSIZ).
>
Ouch. We should be using SVN__STREAM_CHUNK_SIZE there.
> Would you be willing to try a trunk build with the above
> modifications? I would be willing to review patches...
>
How about this one, as a stopgap:
Index: subversion/libsvn_subr/io.c
===================================================================
--- subversion/libsvn_subr/io.c (revision 18760)
+++ subversion/libsvn_subr/io.c (working copy)
@@ -988,7 +988,7 @@
struct apr_md5_ctx_t context;
apr_file_t *f = NULL;
svn_error_t *err;
- char buf[BUFSIZ]; /* What's a good size for a read chunk? */
+ char *buf = apr_palloc(pool, SVN__STREAM_CHUNK_SIZE);
apr_size_t len;
/* ### The apr_md5 functions return apr_status_t, but they only
@@ -999,12 +999,12 @@
SVN_ERR(svn_io_file_open(&f, file, APR_READ, APR_OS_DEFAULT, pool));
- len = sizeof(buf);
+ len = SVN__STREAM_CHUNK_SIZE;
err = svn_io_file_read(f, buf, &len, pool);
while (! err)
{
apr_md5_update(&context, buf, len);
- len = sizeof(buf);
+ len = SVN__STREAM_CHUNK_SIZE;
err = svn_io_file_read(f, buf, &len, pool);
};
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Mar 8 19:11:57 2006