You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by st...@apache.org on 2012/03/03 15:27:51 UTC
svn commit: r1296628 - /subversion/trunk/subversion/svnserve/serve.c
Author: stefan2
Date: Sat Mar 3 14:27:51 2012
New Revision: 1296628
URL: http://svn.apache.org/viewvc?rev=1296628&view=rev
Log:
Make svn ls faster and more streamy on svn://
* subversion/svnserve/serve.c
(get_dir): don't collect entries but send them immediately
Modified:
subversion/trunk/subversion/svnserve/serve.c
Modified: subversion/trunk/subversion/svnserve/serve.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svnserve/serve.c?rev=1296628&r1=1296627&r2=1296628&view=diff
==============================================================================
--- subversion/trunk/subversion/svnserve/serve.c (original)
+++ subversion/trunk/subversion/svnserve/serve.c Sat Mar 3 14:27:51 2012
@@ -1470,7 +1470,7 @@ static svn_error_t *get_dir(svn_ra_svn_c
apr_array_header_t *params, void *baton)
{
server_baton_t *b = baton;
- const char *path, *full_path, *file_path, *cauthor, *cdate;
+ const char *path, *full_path, *file_path, *cdate;
svn_revnum_t rev;
apr_hash_t *entries, *props = NULL, *file_props;
apr_hash_index_t *hi;
@@ -1540,9 +1540,15 @@ static svn_error_t *get_dir(svn_ra_svn_c
if (want_props)
SVN_CMD_ERR(get_props(&props, root, full_path, pool));
- /* Fetch the directory entries if requested. */
+ /* Begin response ... */
+ SVN_ERR(svn_ra_svn_write_tuple(conn, pool, "w(r(!", "success", rev));
+ SVN_ERR(svn_ra_svn_write_proplist(conn, pool, props));
+ SVN_ERR(svn_ra_svn_write_tuple(conn, pool, "!)(!"));
+
+ /* Fetch the directory entries if requested and send them immediately. */
if (want_contents)
{
+ const char *zero_date = svn_time_to_cstring(0, pool);
SVN_CMD_ERR(svn_fs_dir_entries(&entries, root, full_path, pool));
/* Transform the hash table's FS entries into dirents. This probably
@@ -1552,91 +1558,62 @@ static svn_error_t *get_dir(svn_ra_svn_c
{
const char *name = svn__apr_hash_index_key(hi);
svn_fs_dirent_t *fsent = svn__apr_hash_index_val(hi);
- svn_dirent_t *entry;
+
+ svn_dirent_t entry;
+ memset(&entry, 0, sizeof(entry));
svn_pool_clear(subpool);
file_path = svn_fspath__join(full_path, name, subpool);
-
if (! lookup_access(subpool, b, conn, svn_authz_read,
file_path, FALSE))
- {
- apr_hash_set(entries, name, APR_HASH_KEY_STRING, NULL);
- continue;
- }
-
- entry = apr_pcalloc(pool, sizeof(*entry));
+ continue;
if (dirent_fields & SVN_DIRENT_KIND)
- {
- /* kind */
- entry->kind = fsent->kind;
- }
+ entry.kind = fsent->kind;
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,
+ if (entry.kind != svn_node_dir)
+ SVN_CMD_ERR(svn_fs_file_length(&entry.size, root, file_path,
subpool));
- }
if (dirent_fields & SVN_DIRENT_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);
+ entry.has_props = (apr_hash_count(file_props) > 0);
}
+ cdate = NULL;
if ((dirent_fields & SVN_DIRENT_LAST_AUTHOR)
|| (dirent_fields & SVN_DIRENT_TIME)
|| (dirent_fields & SVN_DIRENT_CREATED_REV))
{
/* created_rev, last_author, time */
- SVN_CMD_ERR(svn_repos_get_committed_info(&entry->created_rev,
+ SVN_CMD_ERR(svn_repos_get_committed_info(&entry.created_rev,
&cdate,
- &cauthor, root,
+ &entry.last_author,
+ root,
file_path,
subpool));
- entry->last_author = apr_pstrdup(pool, cauthor);
- if (cdate)
- SVN_CMD_ERR(svn_time_from_cstring(&entry->time, cdate,
- subpool));
- else
- entry->time = (time_t) -1;
}
- /* Store the entry. */
- apr_hash_set(entries, name, APR_HASH_KEY_STRING, entry);
- }
- svn_pool_destroy(subpool);
- }
-
- /* Write out response. */
- SVN_ERR(svn_ra_svn_write_tuple(conn, pool, "w(r(!", "success", rev));
- SVN_ERR(svn_ra_svn_write_proplist(conn, pool, props));
- SVN_ERR(svn_ra_svn_write_tuple(conn, pool, "!)(!"));
- if (want_contents)
- {
- for (hi = apr_hash_first(pool, entries); hi; hi = apr_hash_next(hi))
- {
- const char *name = svn__apr_hash_index_key(hi);
- svn_dirent_t *entry = svn__apr_hash_index_val(hi);
+ if (cdate == NULL)
+ cdate = zero_date;
- cdate = (entry->time == (time_t) -1) ? NULL
- : svn_time_to_cstring(entry->time, pool);
+ /* Send the entry. */
SVN_ERR(svn_ra_svn_write_tuple(conn, pool, "cwnbr(?c)(?c)", name,
- svn_node_kind_to_word(entry->kind),
- (apr_uint64_t) entry->size,
- entry->has_props, entry->created_rev,
- cdate, entry->last_author));
+ svn_node_kind_to_word(entry.kind),
+ (apr_uint64_t) entry.size,
+ entry.has_props, entry.created_rev,
+ cdate, entry.last_author));
}
+ svn_pool_destroy(subpool);
}
- SVN_ERR(svn_ra_svn_write_tuple(conn, pool, "!))"));
- return SVN_NO_ERROR;
+
+ /* Finish response. */
+ return svn_ra_svn_write_tuple(conn, pool, "!))");
}
static svn_error_t *update(svn_ra_svn_conn_t *conn, apr_pool_t *pool,
Re: svn commit: r1296628 - /subversion/trunk/subversion/svnserve/serve.c
Posted by Stefan Fuhrmann <eq...@web.de>.
On 04.03.2012 11:32, Daniel Shahaf wrote:
> stefan2@apache.org wrote on Sat, Mar 03, 2012 at 14:27:51 -0000:
>> Author: stefan2
>> Date: Sat Mar 3 14:27:51 2012
>> New Revision: 1296628
>>
>> URL: http://svn.apache.org/viewvc?rev=1296628&view=rev
>> Log:
>> Make svn ls faster and more streamy on svn://
>>
>> * subversion/svnserve/serve.c
>> (get_dir): don't collect entries but send them immediately
>>
>> Modified:
>> subversion/trunk/subversion/svnserve/serve.c
>>
>> Modified: subversion/trunk/subversion/svnserve/serve.c
>> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svnserve/serve.c?rev=1296628&r1=1296627&r2=1296628&view=diff
>> ==============================================================================
>> --- subversion/trunk/subversion/svnserve/serve.c (original)
>> +++ subversion/trunk/subversion/svnserve/serve.c Sat Mar 3 14:27:51 2012
>> @@ -1470,7 +1470,7 @@ static svn_error_t *get_dir(svn_ra_svn_c
>> apr_array_header_t *params, void *baton)
>> {
>> server_baton_t *b = baton;
>> - const char *path, *full_path, *file_path, *cauthor, *cdate;
>> + const char *path, *full_path, *file_path, *cdate;
>> svn_revnum_t rev;
>> apr_hash_t *entries, *props = NULL, *file_props;
>> apr_hash_index_t *hi;
>> @@ -1540,9 +1540,15 @@ static svn_error_t *get_dir(svn_ra_svn_c
>> if (want_props)
>> SVN_CMD_ERR(get_props(&props, root, full_path, pool));
>>
>> - /* Fetch the directory entries if requested. */
>> + /* Begin response ... */
>> + SVN_ERR(svn_ra_svn_write_tuple(conn, pool, "w(r(!", "success", rev));
>> + SVN_ERR(svn_ra_svn_write_proplist(conn, pool, props));
>> + SVN_ERR(svn_ra_svn_write_tuple(conn, pool, "!)(!"));
>> +
>> + /* Fetch the directory entries if requested and send them immediately. */
>> if (want_contents)
>> {
>> + const char *zero_date = svn_time_to_cstring(0, pool);
>> SVN_CMD_ERR(svn_fs_dir_entries(&entries, root, full_path, pool));
>>
>> /* Transform the hash table's FS entries into dirents. This probably
>> @@ -1552,91 +1558,62 @@ static svn_error_t *get_dir(svn_ra_svn_c
>> {
>> const char *name = svn__apr_hash_index_key(hi);
>> svn_fs_dirent_t *fsent = svn__apr_hash_index_val(hi);
>> - svn_dirent_t *entry;
>> +
>> + svn_dirent_t entry;
>> + memset(&entry, 0, sizeof(entry));
> The old code allocated the struct on the heap, and you do so on the
> stack. But the struct has a _dup() function.
>
> Could the size of the struct change in future versions? If so,
> shouldn't we add a constructor function for it (and use it here)?
>
We don't get the entry from any function call
nor do we pass it to some function. It is simply
a structured chunk of memory that we use
instead of a bunch of independent variables.
IOW, even if the definition of that struct changed
dramatically in other places, this function will
always work once it got compiled -- even if
other libs use incompatible definitions.
-- Stefan^2.
Re: svn commit: r1296628 -
/subversion/trunk/subversion/svnserve/serve.c
Posted by Daniel Shahaf <da...@elego.de>.
stefan2@apache.org wrote on Sat, Mar 03, 2012 at 14:27:51 -0000:
> Author: stefan2
> Date: Sat Mar 3 14:27:51 2012
> New Revision: 1296628
>
> URL: http://svn.apache.org/viewvc?rev=1296628&view=rev
> Log:
> Make svn ls faster and more streamy on svn://
>
> * subversion/svnserve/serve.c
> (get_dir): don't collect entries but send them immediately
>
> Modified:
> subversion/trunk/subversion/svnserve/serve.c
>
> Modified: subversion/trunk/subversion/svnserve/serve.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svnserve/serve.c?rev=1296628&r1=1296627&r2=1296628&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/svnserve/serve.c (original)
> +++ subversion/trunk/subversion/svnserve/serve.c Sat Mar 3 14:27:51 2012
> @@ -1470,7 +1470,7 @@ static svn_error_t *get_dir(svn_ra_svn_c
> apr_array_header_t *params, void *baton)
> {
> server_baton_t *b = baton;
> - const char *path, *full_path, *file_path, *cauthor, *cdate;
> + const char *path, *full_path, *file_path, *cdate;
> svn_revnum_t rev;
> apr_hash_t *entries, *props = NULL, *file_props;
> apr_hash_index_t *hi;
> @@ -1540,9 +1540,15 @@ static svn_error_t *get_dir(svn_ra_svn_c
> if (want_props)
> SVN_CMD_ERR(get_props(&props, root, full_path, pool));
>
> - /* Fetch the directory entries if requested. */
> + /* Begin response ... */
> + SVN_ERR(svn_ra_svn_write_tuple(conn, pool, "w(r(!", "success", rev));
> + SVN_ERR(svn_ra_svn_write_proplist(conn, pool, props));
> + SVN_ERR(svn_ra_svn_write_tuple(conn, pool, "!)(!"));
> +
> + /* Fetch the directory entries if requested and send them immediately. */
> if (want_contents)
> {
> + const char *zero_date = svn_time_to_cstring(0, pool);
> SVN_CMD_ERR(svn_fs_dir_entries(&entries, root, full_path, pool));
>
> /* Transform the hash table's FS entries into dirents. This probably
> @@ -1552,91 +1558,62 @@ static svn_error_t *get_dir(svn_ra_svn_c
> {
> const char *name = svn__apr_hash_index_key(hi);
> svn_fs_dirent_t *fsent = svn__apr_hash_index_val(hi);
> - svn_dirent_t *entry;
> +
> + svn_dirent_t entry;
> + memset(&entry, 0, sizeof(entry));
The old code allocated the struct on the heap, and you do so on the
stack. But the struct has a _dup() function.
Could the size of the struct change in future versions? If so,
shouldn't we add a constructor function for it (and use it here)?
Re: svn commit: r1296628 - Make svn ls faster and more streamy on svn://
Posted by Julian Foad <ju...@btopenworld.com>.
stefan2@apache.org wrote:
> Log:
> Make svn ls faster and more streamy on svn://
>
> * subversion/svnserve/serve.c
> (get_dir): don't collect entries but send them immediately
> + /* Fetch the directory entries if requested and send them immediately. */
> if (want_contents)
> {
> + const char *zero_date = svn_time_to_cstring(0, pool);
This looks wrong. Here you construct a date string something like "1 Jan 1970", to send if no date is available...
> + cdate = NULL;
> if ((dirent_fields & SVN_DIRENT_LAST_AUTHOR)
> || (dirent_fields & SVN_DIRENT_TIME)
> || (dirent_fields & SVN_DIRENT_CREATED_REV))
> {
> /* created_rev, last_author, time */
[...]
> SVN_CMD_ERR(svn_repos_get_committed_info(&entry.created_rev,
> &cdate,
> }
> + if (cdate == NULL)
> + cdate = zero_date;
[...]
> - if (cdate)
> - SVN_CMD_ERR(svn_time_from_cstring(&entry->time, cdate,
> - subpool));
> - else
> - entry->time = (time_t) -1;
> }
[...]
> - /* Write out response. */
> - if (want_contents)
> - {
> - for (hi = apr_hash_first(pool, entries); hi; hi = apr_hash_next(hi))
> - {
> - const char *name = svn__apr_hash_index_key(hi);
> - svn_dirent_t *entry = svn__apr_hash_index_val(hi);
>
> - cdate = (entry->time == (time_t) -1) ? NULL
> - : svn_time_to_cstring(entry->time, pool);
... but the original code used NULL as the value to send if there was no date.
- Julian
Re: svn commit: r1296628 -
/subversion/trunk/subversion/svnserve/serve.c
Posted by Daniel Shahaf <da...@elego.de>.
stefan2@apache.org wrote on Sat, Mar 03, 2012 at 14:27:51 -0000:
> Author: stefan2
> Date: Sat Mar 3 14:27:51 2012
> New Revision: 1296628
>
> URL: http://svn.apache.org/viewvc?rev=1296628&view=rev
> Log:
> Make svn ls faster and more streamy on svn://
>
> * subversion/svnserve/serve.c
> (get_dir): don't collect entries but send them immediately
>
> Modified:
> subversion/trunk/subversion/svnserve/serve.c
>
> Modified: subversion/trunk/subversion/svnserve/serve.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svnserve/serve.c?rev=1296628&r1=1296627&r2=1296628&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/svnserve/serve.c (original)
> +++ subversion/trunk/subversion/svnserve/serve.c Sat Mar 3 14:27:51 2012
> @@ -1470,7 +1470,7 @@ static svn_error_t *get_dir(svn_ra_svn_c
> apr_array_header_t *params, void *baton)
> {
> server_baton_t *b = baton;
> - const char *path, *full_path, *file_path, *cauthor, *cdate;
> + const char *path, *full_path, *file_path, *cdate;
> svn_revnum_t rev;
> apr_hash_t *entries, *props = NULL, *file_props;
> apr_hash_index_t *hi;
> @@ -1540,9 +1540,15 @@ static svn_error_t *get_dir(svn_ra_svn_c
> if (want_props)
> SVN_CMD_ERR(get_props(&props, root, full_path, pool));
>
> - /* Fetch the directory entries if requested. */
> + /* Begin response ... */
> + SVN_ERR(svn_ra_svn_write_tuple(conn, pool, "w(r(!", "success", rev));
> + SVN_ERR(svn_ra_svn_write_proplist(conn, pool, props));
> + SVN_ERR(svn_ra_svn_write_tuple(conn, pool, "!)(!"));
> +
> + /* Fetch the directory entries if requested and send them immediately. */
> if (want_contents)
> {
> + const char *zero_date = svn_time_to_cstring(0, pool);
> SVN_CMD_ERR(svn_fs_dir_entries(&entries, root, full_path, pool));
>
> /* Transform the hash table's FS entries into dirents. This probably
> @@ -1552,91 +1558,62 @@ static svn_error_t *get_dir(svn_ra_svn_c
> {
> const char *name = svn__apr_hash_index_key(hi);
> svn_fs_dirent_t *fsent = svn__apr_hash_index_val(hi);
> - svn_dirent_t *entry;
> +
> + svn_dirent_t entry;
> + memset(&entry, 0, sizeof(entry));
The old code allocated the struct on the heap, and you do so on the
stack. But the struct has a _dup() function.
Could the size of the struct change in future versions? If so,
shouldn't we add a constructor function for it (and use it here)?