On 04.03.2012 11:32, Daniel Shahaf wrote:
> stefan2_at_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.
Received on 2012-03-05 00:36:03 CET