You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Garrett Rooney <ro...@electricjellyfish.net> on 2005/10/26 22:14:07 UTC
[PATCH] improve speed of non-verbose svn list, take 2
Ok, so thanks to the great feedback on the first iteration of this
patch I've now got another version for you guys to take a look at.
Changes between the first version and this one are:
- Removed get_dir2 entry in ra vtable, just modified get_dir instead
- Drop useless get_dir wrappers in ra libs, libsvn_ra takes care of that
- SVN_DIRENT_ALL now defined as ~0
- Updated libsvn_ra_svn/protocol
- Remove useless subpool in ra_dav
- Fixed off by one in ra_dav that broke everything
- Implemented ra_local support
- Changed ra_svn impl to passing a list of words instead of a bitfield
- Used capital letters in log message to make kfogel happy
As usual, comments are more than welcome!
-garrett
Speed up non-verbose 'svn list' commands by a scary amount by allowing
the server to avoid calculating parts of the svn_dirent_t objects that
are never used.
* subversion/libsvn_ra/wrapper_template.h
(compat_get_dir): Update for new version of get_dir, pass SVN_DIRENT_ALL
for the dirent_flags parameter.
* subversion/libsvn_ra/ra_loader.c
(svn_ra_get_dir): Ditto.
(svn_ra_get_dir2): New wrapper function.
* subversion/libsvn_ra/ra_loader.h
(svn_ra__vtable_t): Add a dirent_fields parameter to the get_dir
function.
* subversion/include/svn_ra_svn.h
(SVN_RA_SVN_DIRENT_KIND,
SVN_RA_SVN_DIRENT_SIZE,
SVN_RA_SVN_DIRENT_HAS_PROPS,
SVN_RA_SVN_DIRENT_CREATED_REV,
SVN_RA_SVN_DIRENT_TIME,
SVN_RA_SVN_DIRENT_LAST_AUTHOR): Constants to represent dirent bitfield
values on the wire.
* subversion/include/svn_types.h
(SVN_DIRENT_KIND,
SVN_DIRENT_SIZE,
SVN_DIRENT_HAS_PROPS,
SVN_DIRENT_CREATED_REV,
SVN_DIRENT_TIME,
SVN_DIRENT_LAST_AUTHOR): New bitfield values for each field in a dirent.
(SVN_DIRENT_ALL): Combination of all the other bitfield values.
* subversion/include/svn_client.h
(svn_client_ls4): New function, like svn_client_ls3, but with a dirent
fields parameter.
(svn_client_ls3): Deprecated in favor of svn_client_ls4.
* subversion/include/svn_ra.h
(svn_ra_get_dir2): New version of svn_ra_get_dir, takes a dirent fields
parameter.
(svn_ra_get_dir): Deprecate in favor of svn_ra_get_dir2.
* subversion/libsvn_ra_local/ra_plugin.c
(svn_ra_local__get_dir): Add dirent_fields parameter and use it to
determine which parts of the dirents we need to fill in.
* subversion/libsvn_client/ls.c
(get_dir_contents): Add a dirent_fields parameter, use svn_ra_get_dir2,
update recursive call.
(svn_client_ls4): New list function, adds a dirent_fields parameter which
is passed to get_dir_contents.
(svn_client_ls3): Backwards compat wrapper, calls svn_client_ls4 with the
dirent_fields set to SVN_DIRENT_ALL.
* subversion/clients/cmdline/ls-cmd.c
(svn_cl__ls): Use svn_client_ls4, only asking for the dirent fields we
actually need.
* subversion/libsvn_ra_svn/client.c
(ra_svn_get_dir): Take a dirent_fields parameter and pass it along via an
optional new parameter to the get-dir command that holds a list of
strings representing each field.
* subversion/libsvn_ra_svn/protocol
(get-dir): Update for new dirent fields list.
* subversion/libsvn_ra_dav/ra_dav.h
(svn_ra_dav__get_dir): Add dirent_fields parameter.
* subversion/libsvn_ra_dav/fetch.c
(svn_ra_dav__get_dir): Add dirent_fields parameter and try to limit the
number of properties we ask the server to send back based on it.
* subversion/svnserve/serve.c
(get_dir): Parse new dirent_fields list from the get-dir command and
only fill in the portions of the dirents that the user actually asked
for.
Re: [PATCH] improve speed of non-verbose svn list, take 2
Posted by kf...@collab.net.
Garrett Rooney <ro...@electricjellyfish.net> writes:
> - Used capital letters in log message to make kfogel happy
Did I complain about that? I don't recall doing so... :-)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] improve speed of non-verbose svn list, take 2
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On 10/27/05, Peter N. Lundblad <pe...@famlundblad.se> wrote:
> > get-dir
> > - params: ( path:string [ rev:number ] want-props:bool want-contents:bool )
> > + params: ( path:string [ rev:number ] want-props:bool want-contents:bool
> > + [ dirent-field:string ... ] )
>
> The last part should read "? ( dirent-field:word ...)".
Ahh, ok, my mistake.
> Even better would be to define:
> dirent-field: size | has-props | WHATEVER | word
> and use that. ("word" would allow for future extensibility.)
Yes, I agree, that does make more sense. My current version looks like this:
get-dir
params: ( path:string [ rev:number ] want-props:bool want-contents:bool
? ( field:dirent-field ... ) )
response: ( rev:number props:proplist ( entry:dirent ... ) )]
dirent: ( name:string kind:node-kind size:number has-props:bool
created-rev:number [ created-date:string ]
[ last-author:string ] )
dirent-field: kind | size | has-props | created-rev | time | last-author
| word
Does that seem reasonable?
> > - entry->has_props = (apr_hash_count(file_props) > 0) ? TRUE : FALSE;
> > + if (dirent_fields & SVN_DIRENT_SIZE)
>
> ^ should be HAS_PROPS
Oops. Good catch.
> Else, it looks good to me.
Great, thanks for the review!
-garrett
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] improve speed of non-verbose svn list, take 2
Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
On Wed, 26 Oct 2005, Garrett Rooney wrote:
> Ok, so thanks to the great feedback on the first iteration of this
> patch I've now got another version for you guys to take a look at.
>
I haven't check the DAV part.
> Index: subversion/libsvn_ra_svn/protocol
> ===================================================================
> --- subversion/libsvn_ra_svn/protocol (revision 17044)
> +++ subversion/libsvn_ra_svn/protocol (working copy)
> @@ -252,7 +252,8 @@
> whether an error occurred during the sending of the file.
>
> get-dir
> - params: ( path:string [ rev:number ] want-props:bool want-contents:bool )
> + params: ( path:string [ rev:number ] want-props:bool want-contents:bool
> + [ dirent-field:string ... ] )
The last part should read "? ( dirent-field:word ...)".
Even better would be to define:
dirent-field: size | has-props | WHATEVER | word
and use that. ("word" would allow for future extensibility.)
> response: ( rev:number props:proplist ( entry:dirent ... ) )]
> dirent: ( name:string kind:node-kind size:number has-props:bool
> created-rev:number [ created-date:string ]
> Index: subversion/svnserve/serve.c
> ===================================================================
> --- subversion/svnserve/serve.c (revision 17044)
> +++ subversion/svnserve/serve.c (working copy)
> @@ -1102,30 +1138,48 @@
> file_path = svn_path_join(full_path, name, subpool);
> entry = apr_pcalloc(pool, sizeof(*entry));
>
> - /* kind */
> - entry->kind = fsent->kind;
> + if (dirent_fields & SVN_DIRENT_KIND)
> + {
> + /* kind */
> + entry->kind = fsent->kind;
> + }
>
> - /* size */
> - if (entry->kind == svn_node_dir)
> - entry->size = 0;
> - else
> - SVN_CMD_ERR(svn_fs_file_length(&entry->size, root, file_path,
> - subpool));
> + if (dirent_fields & SVN_DIRENT_SIZE)
> + {
> + /* size */
> + if (entry->kind == svn_node_dir)
> + entry->size = 0;
> + else
> + SVN_CMD_ERR(svn_fs_file_length(&entry->size, root, file_path,
> + subpool));
> + }
>
> - /* has_props */
> - SVN_CMD_ERR(svn_fs_node_proplist(&file_props, root, file_path,
> - subpool));
> - entry->has_props = (apr_hash_count(file_props) > 0) ? TRUE : FALSE;
> + if (dirent_fields & SVN_DIRENT_SIZE)
^ should be HAS_PROPS
> + {
> + /* has_props */
> + SVN_CMD_ERR(svn_fs_node_proplist(&file_props, root, file_path,
> + subpool));
> + entry->has_props = (apr_hash_count(file_props) > 0) ? TRUE
> + : FALSE;
> + }
>
Else, it looks good to me.
Regards,
//Peter
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org