You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Eric Gillespie <ep...@pretzelnet.org> on 2007/08/17 00:47:39 UTC
[PATCH] Update progress while ra-neon spools text delta
[[[
When committing a new gigantic file, ra-neon spends a long time
calculating a text delta and spooling it to disk, during which time
application progress meters are left hanging. Unless we call the
progress callback here, too, so let's do that.
* subversion/libsvn_ra_neon/ra_neon.h
(svn_ra_neon__session_t): Add fields for progress_func and baton.
* subversion/libsvn_ra_neon/session.c
(svn_ra_neon__open): Save progress_func and baton in ras.
* subversion/libsvn_ra_neon/commit.c
(put_baton_t): Add fields for progress bytes, session baton, and pool.
(commit_stream_write): Add bytes written to pb->progress and pass it
to the progress callback.
(commit_apply_txdelta): Set session baton and pool in put_baton.
]]]
Index: subversion/libsvn_ra_neon/session.c
===================================================================
--- subversion/libsvn_ra_neon/session.c (revision 26021)
+++ subversion/libsvn_ra_neon/session.c (working copy)
@@ -782,6 +782,8 @@
ras->callbacks = callbacks;
ras->callback_baton = callback_baton;
ras->compression = compression;
+ ras->progress_baton = callbacks->progress_baton;
+ ras->progress_func = callbacks->progress_func;
/* save config and server group in the auth parameter hash */
svn_auth_set_parameter(ras->callbacks->auth_baton,
SVN_AUTH_PARAM_CONFIG, cfg);
Index: subversion/libsvn_ra_neon/ra_neon.h
===================================================================
--- subversion/libsvn_ra_neon/ra_neon.h (revision 26021)
+++ subversion/libsvn_ra_neon/ra_neon.h (working copy)
@@ -95,6 +95,9 @@
svn_boolean_t compression; /* should we use http compression? */
const char *uuid; /* repository UUID */
+
+ svn_ra_progress_notify_func_t progress_func;
+ void *progress_baton;
} svn_ra_neon__session_t;
Index: subversion/libsvn_ra_neon/commit.c
===================================================================
--- subversion/libsvn_ra_neon/commit.c (revision 26021)
+++ subversion/libsvn_ra_neon/commit.c (working copy)
@@ -105,6 +105,9 @@
apr_file_t *tmpfile; /* may be NULL for content-less file */
svn_stringbuf_t *fname; /* may be NULL for content-less file */
const char *base_checksum; /* hex md5 of base text; may be null */
+ apr_off_t progress;
+ svn_ra_neon__session_t *ras;
+ apr_pool_t *pool;
} put_baton_t;
typedef struct
@@ -1122,6 +1125,8 @@
if (status)
return svn_error_wrap_apr(status,
_("Could not write svndiff to temp file"));
+ pb->progress += *len;
+ pb->ras->progress_func(pb->progress, -1, pb->ras->progress_baton, pb->pool);
return SVN_NO_ERROR;
}
@@ -1138,6 +1143,8 @@
svn_stream_t *stream;
baton = apr_pcalloc(file->pool, sizeof(*baton));
+ baton->ras = file->cc->ras;
+ baton->pool = pool;
file->put_baton = baton;
if (base_checksum)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Update progress while ra-neon spools text delta
Posted by Eric Gillespie <ep...@pretzelnet.org>.
Ping.
Eric Gillespie <ep...@pretzelnet.org> writes:
> Eric Gillespie <ep...@pretzelnet.org> writes:
>
> > Index: subversion/libsvn_ra_neon/commit.c
> > ===================================================================
> > --- subversion/libsvn_ra_neon/commit.c (revision 26021)
> > +++ subversion/libsvn_ra_neon/commit.c (working copy)
> > @@ -1122,6 +1125,8 @@
> > if (status)
> > return svn_error_wrap_apr(status,
> > _("Could not write svndiff to temp file"));
> > + pb->progress += *len;
> > + pb->ras->progress_func(pb->progress, -1, pb->ras->progress_baton, pb->pool
> > );
>
> This needs to test that progress_func is not NULL, obviously. Fixed.
>
> --
> Eric Gillespie <*> epg@pretzelnet.org
--
Eric Gillespie <*> epg@pretzelnet.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Update progress while ra-neon spools text delta
Posted by Eric Gillespie <ep...@pretzelnet.org>.
Eric Gillespie <ep...@pretzelnet.org> writes:
> Index: subversion/libsvn_ra_neon/commit.c
> ===================================================================
> --- subversion/libsvn_ra_neon/commit.c (revision 26021)
> +++ subversion/libsvn_ra_neon/commit.c (working copy)
> @@ -1122,6 +1125,8 @@
> if (status)
> return svn_error_wrap_apr(status,
> _("Could not write svndiff to temp file"));
> + pb->progress += *len;
> + pb->ras->progress_func(pb->progress, -1, pb->ras->progress_baton, pb->pool
> );
This needs to test that progress_func is not NULL, obviously. Fixed.
--
Eric Gillespie <*> epg@pretzelnet.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Update progress while ra-neon spools text delta
Posted by Eric Gillespie <ep...@pretzelnet.org>.
Daniel Rall <dl...@finemaltcoding.com> writes:
> > @@ -1138,6 +1143,8 @@
> > svn_stream_t *stream;
> >
> > baton = apr_pcalloc(file->pool, sizeof(*baton));
> > + baton->ras = file->cc->ras;
> > + baton->pool = pool;
> > file->put_baton = baton;
> >
> > if (base_checksum)
>
> Should we use file->pool instead of the pool parameter passed to
> commit_apply_txdelta()? I think baton's useful lifetime is that of
> commit_apply_txdelta()'s stack frame, but since it's allocated out of
> file->pool rather than pool, I think it would be more correct unless
> we document that put_baton_t.pool should be used only for temporary
> allocations.
We talked about this on IRC. svn_delta.h does not describe the
pool passed to apply_textdelta, but it seems to me that the
svn_stream_create and svn_txdelta_to_svndiff calls here should
also be using file->pool, not the passed-in pool. Or they should
all be using the passed-in pool, I'm not sure; it seems bogus
that some use one and some the other. I suppose it depends
whether the driver clears this pool directly after the call.
I'm testing with Dan's suggested change now, looks good, will
commit soon.
Thanks.
--
Eric Gillespie <*> epg@pretzelnet.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Update progress while ra-neon spools text delta
Posted by Daniel Rall <dl...@finemaltcoding.com>.
Eric, the patch looks good. One minor question below...
On Aug 16, 2007, at 5:47 PM, Eric Gillespie wrote:
...
> --- subversion/libsvn_ra_neon/commit.c (revision 26021)
> +++ subversion/libsvn_ra_neon/commit.c (working copy)
> @@ -105,6 +105,9 @@
> apr_file_t *tmpfile; /* may be NULL for content-less file */
> svn_stringbuf_t *fname; /* may be NULL for content-less file */
> const char *base_checksum; /* hex md5 of base text; may be null */
> + apr_off_t progress;
> + svn_ra_neon__session_t *ras;
> + apr_pool_t *pool;
> } put_baton_t;
>
> typedef struct
> @@ -1122,6 +1125,8 @@
> if (status)
> return svn_error_wrap_apr(status,
> _("Could not write svndiff to temp
> file"));
> + pb->progress += *len;
> + pb->ras->progress_func(pb->progress, -1, pb->ras-
> >progress_baton, pb->pool);
>
> return SVN_NO_ERROR;
> }
> @@ -1138,6 +1143,8 @@
> svn_stream_t *stream;
>
> baton = apr_pcalloc(file->pool, sizeof(*baton));
> + baton->ras = file->cc->ras;
> + baton->pool = pool;
> file->put_baton = baton;
>
> if (base_checksum)
Should we use file->pool instead of the pool parameter passed to
commit_apply_txdelta()? I think baton's useful lifetime is that of
commit_apply_txdelta()'s stack frame, but since it's allocated out of
file->pool rather than pool, I think it would be more correct unless
we document that put_baton_t.pool should be used only for temporary
allocations.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org