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 2016/01/31 10:29:39 UTC

svn commit: r1727785 - /subversion/trunk/subversion/libsvn_ra_svn/marshal.c

Author: stefan2
Date: Sun Jan 31 09:29:39 2016
New Revision: 1727785

URL: http://svn.apache.org/viewvc?rev=1727785&view=rev
Log:
Reduce memory consumption of DAV merge responses (commits).

* subversion/mod_dav_svn/merge.c
  (do_resources): Since the paths in the changes list provided by the FS
                  are already unique, we only need to track thoes that
                  might clash with ones we add ourselves. 

Modified:
    subversion/trunk/subversion/libsvn_ra_svn/marshal.c

Modified: subversion/trunk/subversion/libsvn_ra_svn/marshal.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_svn/marshal.c?rev=1727785&r1=1727784&r2=1727785&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_svn/marshal.c (original)
+++ subversion/trunk/subversion/libsvn_ra_svn/marshal.c Sun Jan 31 09:29:39 2016
@@ -2804,16 +2804,16 @@ changed_path_flags(svn_node_kind_t node_
 svn_error_t *
 svn_ra_svn__write_data_log_changed_path(svn_ra_svn_conn_t *conn,
                                         apr_pool_t *pool,
-                                        const char *path,
+                                        const svn_string_t *path,
                                         char action,
-                                        const char *copyfrom_path,
+                                        const svn_string_t *copyfrom_path,
                                         svn_revnum_t copyfrom_rev,
                                         svn_node_kind_t node_kind,
                                         svn_boolean_t text_modified,
                                         svn_boolean_t props_modified)
 {
-  apr_size_t path_len = strlen(path);
-  apr_size_t copyfrom_len = copyfrom_path ? strlen(copyfrom_path) : 0;
+  apr_size_t path_len = path->len;
+  apr_size_t copyfrom_len = copyfrom_path->len;
   const svn_string_t *flags_str = changed_path_flags(node_kind,
                                                      text_modified,
                                                      props_modified);
@@ -2845,7 +2845,7 @@ svn_ra_svn__write_data_log_changed_path(
       p[1] = ' ';
 
       /* Write path. */
-      p = write_ncstring_quick(p + 2, path, path_len);
+      p = write_ncstring_quick(p + 2, path->data, path_len);
 
       /* Action */
       p[0] = action;
@@ -2853,10 +2853,10 @@ svn_ra_svn__write_data_log_changed_path(
       p[2] = '(';
 
       /* Copy-from info (if given) */
-      if (copyfrom_path)
+      if (copyfrom_len)
         {
           p[3] = ' ';
-          p = write_ncstring_quick(p + 4, copyfrom_path, copyfrom_len);
+          p = write_ncstring_quick(p + 4, copyfrom_path->data, copyfrom_len);
           p += svn__ui64toa(p, copyfrom_rev);
         }
       else
@@ -2873,11 +2873,13 @@ svn_ra_svn__write_data_log_changed_path(
       /* Standard code path (fallback). */
       SVN_ERR(write_tuple_start_list(conn, pool));
 
-      SVN_ERR(svn_ra_svn__write_ncstring(conn, pool, path, path_len));
+      SVN_ERR(svn_ra_svn__write_ncstring(conn, pool, path->data, path_len));
       SVN_ERR(writebuf_writechar(conn, pool, action));
       SVN_ERR(writebuf_writechar(conn, pool, ' '));
       SVN_ERR(write_tuple_start_list(conn, pool));
-      SVN_ERR(write_tuple_cstring_opt(conn, pool, copyfrom_path));
+      if (copyfrom_len)
+        SVN_ERR(svn_ra_svn__write_ncstring(conn, pool, copyfrom_path->data,
+                                           copyfrom_len));
       SVN_ERR(write_tuple_revision_opt(conn, pool, copyfrom_rev));
       SVN_ERR(write_tuple_end_list(conn, pool));
       SVN_ERR(write_tuple_start_list(conn, pool));



RE: svn commit: r1727785 - /subversion/trunk/subversion/libsvn_ra_svn/marshal.c

Posted by Bert Huijben <be...@qqmail.nl>.

> -----Original Message-----
> From: Stefan Fuhrmann [mailto:stefan2@apache.org]
> Sent: maandag 1 februari 2016 07:29
> To: dev@subversion.apache.org
> Subject: Re: svn commit: r1727785 -
> /subversion/trunk/subversion/libsvn_ra_svn/marshal.c
> 
> 
> 
> On 31.01.2016 17:45, Bert Huijben wrote:
> >
> >
> >> -----Original Message-----
> >> From: stefan2@apache.org [mailto:stefan2@apache.org]
> >> Sent: zondag 31 januari 2016 10:30
> >> To: commits@subversion.apache.org
> >> Subject: svn commit: r1727785 -
> >> /subversion/trunk/subversion/libsvn_ra_svn/marshal.c
> >>
> >> Author: stefan2
> >> Date: Sun Jan 31 09:29:39 2016
> >> New Revision: 1727785
> >>
> >> URL: http://svn.apache.org/viewvc?rev=1727785&view=rev
> >> Log:
> >> Reduce memory consumption of DAV merge responses (commits).
> >>
> >> * subversion/mod_dav_svn/merge.c
> >>    (do_resources): Since the paths in the changes list provided by the FS
> >>                    are already unique, we only need to track thoes that
> >>                    might clash with ones we add ourselves.
> >
> > Where is the memory reduction?
> >
> > I just see an argument type change in a different function (which by itself
> looks like a nice code cleanup)?
> >
> > I think this commit has the wrong log message.
> 
> Nope, wrong file. Got reverted & fixed.
> I tried to update the commit message
> yesterday but it told me that I'm
> not allowed to.  I'll try to fix it
> tonight.

I (still) have the same problem, but somehow Philip was able to change some logmessages today.

	Bert


Re: svn commit: r1727785 - /subversion/trunk/subversion/libsvn_ra_svn/marshal.c

Posted by Stefan Fuhrmann <st...@apache.org>.

On 31.01.2016 17:45, Bert Huijben wrote:
>
>
>> -----Original Message-----
>> From: stefan2@apache.org [mailto:stefan2@apache.org]
>> Sent: zondag 31 januari 2016 10:30
>> To: commits@subversion.apache.org
>> Subject: svn commit: r1727785 -
>> /subversion/trunk/subversion/libsvn_ra_svn/marshal.c
>>
>> Author: stefan2
>> Date: Sun Jan 31 09:29:39 2016
>> New Revision: 1727785
>>
>> URL: http://svn.apache.org/viewvc?rev=1727785&view=rev
>> Log:
>> Reduce memory consumption of DAV merge responses (commits).
>>
>> * subversion/mod_dav_svn/merge.c
>>    (do_resources): Since the paths in the changes list provided by the FS
>>                    are already unique, we only need to track thoes that
>>                    might clash with ones we add ourselves.
>
> Where is the memory reduction?
>
> I just see an argument type change in a different function (which by itself looks like a nice code cleanup)?
>
> I think this commit has the wrong log message.

Nope, wrong file. Got reverted & fixed.
I tried to update the commit message
yesterday but it told me that I'm
not allowed to.  I'll try to fix it
tonight.

-- Stefan^2.

RE: svn commit: r1727785 - /subversion/trunk/subversion/libsvn_ra_svn/marshal.c

Posted by Bert Huijben <be...@qqmail.nl>.

> -----Original Message-----
> From: stefan2@apache.org [mailto:stefan2@apache.org]
> Sent: zondag 31 januari 2016 10:30
> To: commits@subversion.apache.org
> Subject: svn commit: r1727785 -
> /subversion/trunk/subversion/libsvn_ra_svn/marshal.c
> 
> Author: stefan2
> Date: Sun Jan 31 09:29:39 2016
> New Revision: 1727785
> 
> URL: http://svn.apache.org/viewvc?rev=1727785&view=rev
> Log:
> Reduce memory consumption of DAV merge responses (commits).
> 
> * subversion/mod_dav_svn/merge.c
>   (do_resources): Since the paths in the changes list provided by the FS
>                   are already unique, we only need to track thoes that
>                   might clash with ones we add ourselves.

Where is the memory reduction?

I just see an argument type change in a different function (which by itself looks like a nice code cleanup)?

I think this commit has the wrong log message.

	Bert
> 
> Modified:
>     subversion/trunk/subversion/libsvn_ra_svn/marshal.c
> 
> Modified: subversion/trunk/subversion/libsvn_ra_svn/marshal.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_svn/
> marshal.c?rev=1727785&r1=1727784&r2=1727785&view=diff
> ==========================================================
> ====================
> --- subversion/trunk/subversion/libsvn_ra_svn/marshal.c (original)
> +++ subversion/trunk/subversion/libsvn_ra_svn/marshal.c Sun Jan 31
> 09:29:39 2016
> @@ -2804,16 +2804,16 @@ changed_path_flags(svn_node_kind_t node_
>  svn_error_t *
>  svn_ra_svn__write_data_log_changed_path(svn_ra_svn_conn_t *conn,
>                                          apr_pool_t *pool,
> -                                        const char *path,
> +                                        const svn_string_t *path,
>                                          char action,
> -                                        const char *copyfrom_path,
> +                                        const svn_string_t *copyfrom_path,
>                                          svn_revnum_t copyfrom_rev,
>                                          svn_node_kind_t node_kind,
>                                          svn_boolean_t text_modified,
>                                          svn_boolean_t props_modified)
>  {
> -  apr_size_t path_len = strlen(path);
> -  apr_size_t copyfrom_len = copyfrom_path ? strlen(copyfrom_path) : 0;
> +  apr_size_t path_len = path->len;
> +  apr_size_t copyfrom_len = copyfrom_path->len;
>    const svn_string_t *flags_str = changed_path_flags(node_kind,
>                                                       text_modified,
>                                                       props_modified);
> @@ -2845,7 +2845,7 @@ svn_ra_svn__write_data_log_changed_path(
>        p[1] = ' ';
> 
>        /* Write path. */
> -      p = write_ncstring_quick(p + 2, path, path_len);
> +      p = write_ncstring_quick(p + 2, path->data, path_len);
> 
>        /* Action */
>        p[0] = action;
> @@ -2853,10 +2853,10 @@ svn_ra_svn__write_data_log_changed_path(
>        p[2] = '(';
> 
>        /* Copy-from info (if given) */
> -      if (copyfrom_path)
> +      if (copyfrom_len)
>          {
>            p[3] = ' ';
> -          p = write_ncstring_quick(p + 4, copyfrom_path, copyfrom_len);
> +          p = write_ncstring_quick(p + 4, copyfrom_path->data, copyfrom_len);
>            p += svn__ui64toa(p, copyfrom_rev);
>          }
>        else
> @@ -2873,11 +2873,13 @@ svn_ra_svn__write_data_log_changed_path(
>        /* Standard code path (fallback). */
>        SVN_ERR(write_tuple_start_list(conn, pool));
> 
> -      SVN_ERR(svn_ra_svn__write_ncstring(conn, pool, path, path_len));
> +      SVN_ERR(svn_ra_svn__write_ncstring(conn, pool, path->data,
> path_len));
>        SVN_ERR(writebuf_writechar(conn, pool, action));
>        SVN_ERR(writebuf_writechar(conn, pool, ' '));
>        SVN_ERR(write_tuple_start_list(conn, pool));
> -      SVN_ERR(write_tuple_cstring_opt(conn, pool, copyfrom_path));
> +      if (copyfrom_len)
> +        SVN_ERR(svn_ra_svn__write_ncstring(conn, pool, copyfrom_path-
> >data,
> +                                           copyfrom_len));
>        SVN_ERR(write_tuple_revision_opt(conn, pool, copyfrom_rev));
>        SVN_ERR(write_tuple_end_list(conn, pool));
>        SVN_ERR(write_tuple_start_list(conn, pool));
>