You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Chia-liang Kao <cl...@clkao.org> on 2003/08/01 23:16:14 UTC
diff -r N:M URL over ra_rav performance
Hi,
currently svn diff -r N:M URL over ra fetches two versions of
every changed file. this is terrible for small changes of
large files. so as merging.
I'm trying to fix it with getting ra->do_diff M as textdelta.
I've moved delta_base out of custom_get_request, and let
simple_fetch_file take the baserev from ELEM_open_directory.
also added a always_delta flag to make_reporter for do_diff.
now the problem is to build the !svn/ver/ url with the right way.
but ra_dav does not have the root collection info, so i'm
tempted to modify dir->vsn_url, which is really not clean.
any suggestions?
also this seems to be a case of #518
Cheers,
CLK
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] diff/merge -r N:M URL over ra_rav performance
Posted by Chia-liang Kao <cl...@clkao.org>.
heh, Thanks for the quick summary.
after some more looking at the repos_diff_editor and ra_dav,
turned out vsn_url _are_ passed to the diff editor via
change_dir_prop. so in open_file, we actually already have
that information in pb's propchange. but it's just hard to
pass it around cleanly to make ra's get_wc_prop aware of it.
Cheers,
CLK
On Sat, Aug 02, 2003 at 12:06:38PM -0500, Ben Collins-Sussman wrote:
> Chia-liang and I had a chat about this in IRC. I told him that his
> patch would be instantly rejected, because libsvn_ra_dav isn't allowed
> to artifically construct version-resource-urls. That's a major
> violation of DeltaV.
>
> Instead, I suggested a new strategy: have the repos-diff-editor use
> the first RA session to fetch a fulltext (RA->get_file(), and also
> capture the VR-URL property that comes back. Then make that VR-URL
> available to the 2nd RA session via a get-wc-prop callback.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] diff/merge -r N:M URL over ra_rav performance
Posted by Ben Collins-Sussman <su...@collab.net>.
Chia-liang Kao <cl...@clkao.org> writes:
> Make svn diff/merge -r N:M retrieve N as fulltext and M as delta
> over ra_dav. This saves the bandwidth and speeds up 30-50%
> depending on change size and network speed.
>
> * libsvn_ra_dav/fetch.c:
> (svn_ra_dav__do_*): pass in new reporter baton field
> "always_delta".
> (custom_get_request): take computed delta_base.
> (simple_fetch_file): take baserev, generate delta_base
> url if baserev is valid, otherwise use get_delta_base.
> (svn_ra_dav__get_file): adjust parameter for
> custom_get_request.
> (start_element):
> (ELEM_open_directory): store baserev in current dir_item.
> (ELEM_fetch_file): call simple_fetch_file with baserev
> if always_delta is set.
> (end_element: ELEM_add_file): adjust parameter for
> simple_fetch_file.
Chia-liang and I had a chat about this in IRC. I told him that his
patch would be instantly rejected, because libsvn_ra_dav isn't allowed
to artifically construct version-resource-urls. That's a major
violation of DeltaV.
Instead, I suggested a new strategy: have the repos-diff-editor use
the first RA session to fetch a fulltext (RA->get_file(), and also
capture the VR-URL property that comes back. Then make that VR-URL
available to the 2nd RA session via a get-wc-prop callback.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
[PATCH] diff/merge -r N:M URL over ra_rav performance
Posted by Chia-liang Kao <cl...@clkao.org>.
forgot the message body itself, here it is :)
log message:
Make svn diff/merge -r N:M retrieve N as fulltext and M as delta
over ra_dav. This saves the bandwidth and speeds up 30-50%
depending on change size and network speed.
* libsvn_ra_dav/fetch.c:
(svn_ra_dav__do_*): pass in new reporter baton field
"always_delta".
(custom_get_request): take computed delta_base.
(simple_fetch_file): take baserev, generate delta_base
url if baserev is valid, otherwise use get_delta_base.
(svn_ra_dav__get_file): adjust parameter for
custom_get_request.
(start_element):
(ELEM_open_directory): store baserev in current dir_item.
(ELEM_fetch_file): call simple_fetch_file with baserev
if always_delta is set.
(end_element: ELEM_add_file): adjust parameter for
simple_fetch_file.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
[PATCH] diff -r N:M URL over ra_rav performance
Posted by Chia-liang Kao <cl...@clkao.org>.
On Sat, Aug 02, 2003 at 07:16:14AM +0800, Chia-liang Kao wrote:
> now the problem is to build the !svn/ver/ url with the right way.
>
> but ra_dav does not have the root collection info, so i'm
> tempted to modify dir->vsn_url, which is really not clean.
> any suggestions?
here is the patch. some live test results of
svn diff -r 6636:6639 http://svn.collab.net/repos/svn/trunk
with patch:
0.040u 0.015s 0:29.92 0.1% 1472+1249k 0+0io 2pf+0w
without patch:
0.045u 0.015s 0:46.05 0.1% 1622+1168k 0+0io 2pf+0w
log message:
* libsvn_ra_dav/fetch.c:
(svn_ra_dav__do_*): pass in new reporter baton field
"always_delta".
(custom_get_request): take computed delta_base.
(simple_fetch_file): take baserev, generate delta_base
url if baserev is valid, otherwise use get_delta_base.
(svn_ra_dav__get_file): adjust parameter for
custom_get_request.
(start_element):
(ELEM_open_directory): store baserev in current dir_item.
(ELEM_fetch_file): call simple_fetch_file with baserev
if always_delta is set.
(end_element: ELEM_add_file): adjust parameter for
simple_fetch_file.
Index: subversion/libsvn_ra_dav/fetch.c
===================================================================
--- subversion/libsvn_ra_dav/fetch.c (revision 6638)
+++ subversion/libsvn_ra_dav/fetch.c (working copy)
@@ -114,6 +114,9 @@
/* The version resource URL for this directory. */
const char *vsn_url;
+ /* The base revision for this directory. */
+ svn_revnum_t baserev;
+
/* A buffer which stores the relative directory name. We also use this
for temporary construction of relative file names. */
svn_stringbuf_t *pathbuf;
@@ -165,6 +168,7 @@
client should ignore the new wcprop, because the client knows
it's not really updating the top level directory. */
const char *target;
+ svn_revnum_t always_delta;
svn_error_t *err;
@@ -340,11 +344,10 @@
void *subctx,
svn_ra_get_wc_prop_func_t get_wc_prop,
void *cb_baton,
- svn_boolean_t use_base,
+ const char *delta_base,
apr_pool_t *pool)
{
custom_get_ctx_t cgc = { 0 };
- const char *delta_base;
ne_request *req;
ne_decompress *decompress;
svn_error_t *err;
@@ -352,19 +355,6 @@
int decompress_rv;
svn_ra_session_t *ras = ne_get_session_private(sess, SVN_RA_NE_SESSION_ID);
- if (use_base)
- {
- /* See if we can get a version URL for this resource. This will
- refer to what we already have in the working copy, thus we
- can get a diff against this particular resource. */
- SVN_ERR( get_delta_base(&delta_base, relpath,
- get_wc_prop, cb_baton, pool) );
- }
- else
- {
- delta_base = NULL;
- }
-
req = ne_request_create(sess, "GET", url);
if (req == NULL)
{
@@ -542,6 +532,7 @@
const char *url,
const char *relpath,
svn_boolean_t text_deltas,
+ svn_revnum_t baserev,
void *file_baton,
const char *base_checksum,
const svn_delta_editor_t *editor,
@@ -550,6 +541,7 @@
apr_pool_t *pool)
{
file_read_ctx_t frc = { 0 };
+ const char *delta_base;
SVN_ERR_W( (*editor->apply_textdelta)(file_baton,
base_checksum,
@@ -567,10 +559,29 @@
frc.pool = pool;
+ if (baserev != SVN_INVALID_REVNUM)
+ {
+ const char res_vsn[] = "/!svn/ver/";
+ char *root = strstr(url, "/!svn/ver/"), *path;
+
+ if (!root)
+ return svn_error_create(SVN_ERR_BAD_URL, NULL,
+ "can't parse version url");
+ path = strchr (root + sizeof (res_vsn) - 1, '/');
+ delta_base = apr_pstrcat (pool, apr_pstrndup (pool, url,
+ ((int)root - (int)url)),
+ res_vsn,
+ apr_ltoa(pool, baserev), "/",
+ path, NULL);
+ }
+ else
+ SVN_ERR( get_delta_base(&delta_base, relpath,
+ get_wc_prop, cb_baton, pool) );
+
SVN_ERR( custom_get_request(sess, url, relpath,
fetch_file_reader, &frc,
get_wc_prop, cb_baton,
- TRUE, pool) );
+ delta_base, pool) );
/* close the handler, since the file reading completed successfully. */
SVN_ERR( (*frc.handler)(NULL, frc.handler_baton) );
@@ -779,7 +790,7 @@
get_file_reader, &fwc,
ras->callbacks->get_wc_prop,
ras->callback_baton,
- FALSE, pool) );
+ NULL, pool) );
if (fwc.do_checksum)
{
@@ -1437,6 +1448,7 @@
/* Property fetching is NOT implied in replacement. */
TOP_DIR(rb).fetch_props = FALSE;
+ TOP_DIR(rb).baserev = base;
break;
case ELEM_add_directory:
@@ -1629,6 +1641,8 @@
rb->href->data,
TOP_DIR(rb).pathbuf->data,
rb->fetch_content,
+ rb->always_delta ?
+ TOP_DIR(rb).baserev : SVN_INVALID_REVNUM,
rb->file_baton,
base_checksum,
rb->editor,
@@ -1775,6 +1789,7 @@
rb->href->data,
TOP_DIR(rb).pathbuf->data,
rb->fetch_content,
+ SVN_INVALID_REVNUM,
rb->file_baton,
NULL, /* no base checksum in an add */
rb->editor,
@@ -2125,6 +2140,7 @@
svn_boolean_t recurse,
svn_boolean_t ignore_ancestry,
svn_boolean_t resource_walk,
+ svn_boolean_t always_delta,
const svn_delta_editor_t *editor,
void *edit_baton,
svn_boolean_t fetch_content,
@@ -2140,6 +2156,7 @@
rb = apr_pcalloc(pool, sizeof(*rb));
rb->ras = ras;
+ rb->always_delta = always_delta,
rb->editor = editor;
rb->edit_baton = edit_baton;
rb->fetch_content = fetch_content;
@@ -2300,6 +2317,7 @@
recurse,
FALSE,
FALSE,
+ FALSE,
wc_update,
wc_update_baton,
TRUE, /* fetch_content */
@@ -2325,6 +2343,7 @@
recurse,
FALSE,
FALSE,
+ FALSE,
wc_status,
wc_status_baton,
FALSE, /* fetch_content */
@@ -2352,6 +2371,7 @@
recurse,
TRUE,
TRUE,
+ FALSE,
wc_update,
wc_update_baton,
TRUE, /* fetch_content */
@@ -2380,6 +2400,7 @@
recurse,
ignore_ancestry,
FALSE,
+ TRUE,
wc_diff,
wc_diff_baton,
TRUE, /* fetch_content */
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org