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 2015/09/04 21:42:43 UTC

svn commit: r1701318 - in /subversion/trunk/subversion: include/private/svn_ra_svn_private.h libsvn_ra_svn/client.c libsvn_ra_svn/marshal.c svnserve/serve.c

Author: stefan2
Date: Fri Sep  4 19:42:42 2015
New Revision: 1701318

URL: http://svn.apache.org/r1701318
Log:
Fine-tune ra_svn's item type definition: Remove a level of indirection
from the LIST union element.

This saves an extra allocation per list and the space for one pointer
per list at the expense of adding space of 8 bytes per item.  In total,
a 10..20% increase in RAM usage for typical protocol data.

* subversion/include/private/svn_ra_svn_private.h
  (SVN_RA_SVN__LIST_ITEM): Allow for "&" at the start of the list argument.
  (svn_ra_svn__item_t): Instantiate the list directly instead of using
                        a pointer.

* subversion/libsvn_ra_svn/client.c
  (parse_prop_diffs,
   parse_iproplist,
   ra_svn_get_dir,
   perform_ra_svn_log,
   perform_get_location_segments,
   ra_svn_lock,
   ra_svn_unlock,
   ra_svn_get_locks): Update all references to the list union element.

* subversion/libsvn_ra_svn/marshal.c
  (svn_ra_svn__to_public_item,
   svn_ra_svn__to_private_item): Same.
  (read_item): No longer allocate the list struct but simply initialize it.
  (vparse_tuple,
   svn_ra_svn__read_tuple,
   svn_ra_svn__parse_proplist,
   svn_ra_svn__handle_failure_status,
   svn_ra_svn__read_list): Update all references to the list union element.

* subversion/svnserve/serve.c
  (add_lock_tokens,
   unlock_paths,
   lock_many,
   unlock_many): Same.

Modified:
    subversion/trunk/subversion/include/private/svn_ra_svn_private.h
    subversion/trunk/subversion/libsvn_ra_svn/client.c
    subversion/trunk/subversion/libsvn_ra_svn/marshal.c
    subversion/trunk/subversion/svnserve/serve.c

Modified: subversion/trunk/subversion/include/private/svn_ra_svn_private.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/private/svn_ra_svn_private.h?rev=1701318&r1=1701317&r2=1701318&view=diff
==============================================================================
--- subversion/trunk/subversion/include/private/svn_ra_svn_private.h (original)
+++ subversion/trunk/subversion/include/private/svn_ra_svn_private.h Fri Sep  4 19:42:42 2015
@@ -49,7 +49,7 @@ typedef struct svn_ra_svn__list_t
 
 /* List element access macro.  This is for transitional usage only.
  * Once svn_ra_svn__list_t is finalized, this macro will become obsolete. */
-#define SVN_RA_SVN__LIST_ITEM(list, idx) list->items[idx]
+#define SVN_RA_SVN__LIST_ITEM(list, idx) (list)->items[idx]
 
 /** Memory representation of an on-the-wire data item. */
 typedef struct svn_ra_svn__item_t
@@ -62,7 +62,7 @@ typedef struct svn_ra_svn__item_t
     apr_uint64_t number;
     svn_string_t *string;
     const char *word;
-    svn_ra_svn__list_t *list;
+    svn_ra_svn__list_t list;
   } u;
 } svn_ra_svn__item_t;
 

Modified: subversion/trunk/subversion/libsvn_ra_svn/client.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_svn/client.c?rev=1701318&r1=1701317&r2=1701318&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_svn/client.c (original)
+++ subversion/trunk/subversion/libsvn_ra_svn/client.c Fri Sep  4 19:42:42 2015
@@ -204,8 +204,8 @@ static svn_error_t *parse_prop_diffs(con
         return svn_error_create(SVN_ERR_RA_SVN_MALFORMED_DATA, NULL,
                                 _("Prop diffs element not a list"));
       prop = apr_array_push(*diffs);
-      SVN_ERR(svn_ra_svn__parse_tuple(elt->u.list, pool, "c(?s)", &prop->name,
-                                      &prop->value));
+      SVN_ERR(svn_ra_svn__parse_tuple(&elt->u.list, pool, "c(?s)",
+                                      &prop->name, &prop->value));
     }
   return SVN_NO_ERROR;
 }
@@ -1228,7 +1228,7 @@ parse_iproplist(apr_array_header_t **inh
 
       svn_pool_clear(iterpool);
 
-      SVN_ERR(svn_ra_svn__parse_tuple(elt->u.list, iterpool, "cl",
+      SVN_ERR(svn_ra_svn__parse_tuple(&elt->u.list, iterpool, "cl",
                                       &parent_rel_path, &iprop_list));
       SVN_ERR(svn_ra_svn__parse_proplist(iprop_list, iterpool, &iprops));
       new_iprop->path_or_url = svn_path_url_add_component2(repos_root_url,
@@ -1389,7 +1389,7 @@ static svn_error_t *ra_svn_get_dir(svn_r
       if (elt->kind != SVN_RA_SVN_LIST)
         return svn_error_create(SVN_ERR_RA_SVN_MALFORMED_DATA, NULL,
                                 _("Dirlist element not a list"));
-      SVN_ERR(svn_ra_svn__parse_tuple(elt->u.list, pool, "cwnbr(?c)(?c)",
+      SVN_ERR(svn_ra_svn__parse_tuple(&elt->u.list, pool, "cwnbr(?c)(?c)",
                                       &name, &kind, &size, &has_props,
                                       &crev, &cdate, &cauthor));
 
@@ -1481,7 +1481,7 @@ static svn_error_t *ra_svn_get_mergeinfo
           if (elt->kind != SVN_RA_SVN_LIST)
             return svn_error_create(SVN_ERR_RA_SVN_MALFORMED_DATA, NULL,
                                     _("Mergeinfo element is not a list"));
-          SVN_ERR(svn_ra_svn__parse_tuple(elt->u.list, pool, "cc",
+          SVN_ERR(svn_ra_svn__parse_tuple(&elt->u.list, pool, "cc",
                                           &path, &to_parse));
           SVN_ERR(svn_mergeinfo_parse(&for_path, to_parse, pool));
           /* Correct for naughty servers that send "relative" paths
@@ -1700,7 +1700,7 @@ perform_ra_svn_log(svn_error_t **outer_e
       if (item->kind != SVN_RA_SVN_LIST)
         return svn_error_create(SVN_ERR_RA_SVN_MALFORMED_DATA, NULL,
                                 _("Log entry not a list"));
-      SVN_ERR(svn_ra_svn__parse_tuple(item->u.list, iterpool,
+      SVN_ERR(svn_ra_svn__parse_tuple(&item->u.list, iterpool,
                                       "lr(?s)(?s)(?s)?BBnl?B",
                                       &cplist, &rev, &author, &date,
                                       &message, &has_children_param,
@@ -1747,7 +1747,7 @@ perform_ra_svn_log(svn_error_t **outer_e
               if (elt->kind != SVN_RA_SVN_LIST)
                 return svn_error_create(SVN_ERR_RA_SVN_MALFORMED_DATA, NULL,
                                         _("Changed-path entry not a list"));
-              SVN_ERR(svn_ra_svn__read_data_log_changed_entry(elt->u.list,
+              SVN_ERR(svn_ra_svn__read_data_log_changed_entry(&elt->u.list,
                                               &cpath, &action, &copy_path,
                                               &copy_rev, &kind_str,
                                               &text_mods, &prop_mods));
@@ -1978,7 +1978,7 @@ static svn_error_t *ra_svn_get_locations
                                 _("Location entry not a list"));
       else
         {
-          SVN_ERR(svn_ra_svn__parse_tuple(item->u.list, pool, "rc",
+          SVN_ERR(svn_ra_svn__parse_tuple(&item->u.list, pool, "rc",
                                           &revision, &ret_path));
           ret_path = svn_fspath__canonicalize(ret_path, pool);
           apr_hash_set(*locations, apr_pmemdup(pool, &revision,
@@ -2037,7 +2037,7 @@ perform_get_location_segments(svn_error_
         {
           svn_location_segment_t *segment = apr_pcalloc(iterpool,
                                                         sizeof(*segment));
-          SVN_ERR(svn_ra_svn__parse_tuple(item->u.list, iterpool, "rr(?c)",
+          SVN_ERR(svn_ra_svn__parse_tuple(&item->u.list, iterpool, "rr(?c)",
                                           &range_start, &range_end, &ret_path));
           if (! (SVN_IS_VALID_REVNUM(range_start)
                  && SVN_IS_VALID_REVNUM(range_end)))
@@ -2139,7 +2139,7 @@ static svn_error_t *ra_svn_get_file_revs
         return svn_error_create(SVN_ERR_RA_SVN_MALFORMED_DATA, NULL,
                                 _("Revision entry not a list"));
 
-      SVN_ERR(svn_ra_svn__parse_tuple(item->u.list, rev_pool,
+      SVN_ERR(svn_ra_svn__parse_tuple(&item->u.list, rev_pool,
                                       "crll?B", &p, &rev, &rev_proplist,
                                       &proplist, &merged_rev_param));
       p = svn_fspath__canonicalize(p, rev_pool);
@@ -2407,7 +2407,7 @@ static svn_error_t *ra_svn_lock(svn_ra_s
         return svn_error_create(SVN_ERR_RA_SVN_MALFORMED_DATA, NULL,
                                 _("Lock response not a list"));
 
-      SVN_ERR(svn_ra_svn__parse_tuple(elt->u.list, iterpool, "wl", &status,
+      SVN_ERR(svn_ra_svn__parse_tuple(&elt->u.list, iterpool, "wl", &status,
                                       &list));
 
       if (strcmp(status, "failure") == 0)
@@ -2535,7 +2535,7 @@ static svn_error_t *ra_svn_unlock(svn_ra
         return svn_error_create(SVN_ERR_RA_SVN_MALFORMED_DATA, NULL,
                                 _("Unlock response not a list"));
 
-      SVN_ERR(svn_ra_svn__parse_tuple(elt->u.list, iterpool, "wl", &status,
+      SVN_ERR(svn_ra_svn__parse_tuple(&elt->u.list, iterpool, "wl", &status,
                                       &list));
 
       if (strcmp(status, "failure") == 0)
@@ -2661,7 +2661,7 @@ static svn_error_t *ra_svn_get_locks(svn
       if (elt->kind != SVN_RA_SVN_LIST)
         return svn_error_create(SVN_ERR_RA_SVN_MALFORMED_DATA, NULL,
                                 _("Lock element not a list"));
-      SVN_ERR(parse_lock(elt->u.list, pool, &lock));
+      SVN_ERR(parse_lock(&elt->u.list, pool, &lock));
 
       /* Filter out unwanted paths.  Since Subversion only allows
          locks on files, we can treat depth=immediates the same as

Modified: subversion/trunk/subversion/libsvn_ra_svn/marshal.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_svn/marshal.c?rev=1701318&r1=1701317&r2=1701318&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_svn/marshal.c (original)
+++ subversion/trunk/subversion/libsvn_ra_svn/marshal.c Fri Sep  4 19:42:42 2015
@@ -100,7 +100,7 @@ svn_ra_svn__to_public_item(svn_ra_svn_it
         target->u.word = source->u.word;
         break;
       case SVN_RA_SVN_LIST:
-        target->u.list = svn_ra_svn__to_public_array(source->u.list,
+        target->u.list = svn_ra_svn__to_public_array(&source->u.list,
                                                      result_pool);
         break;
     }
@@ -143,8 +143,8 @@ svn_ra_svn__to_private_item(svn_ra_svn__
         target->u.word = source->u.word;
         break;
       case SVN_RA_SVN_LIST:
-        target->u.list = svn_ra_svn__to_private_array(source->u.list,
-                                                      result_pool);
+        target->u.list = *svn_ra_svn__to_private_array(source->u.list,
+                                                       result_pool);
         break;
     }
 }
@@ -1300,8 +1300,6 @@ static svn_error_t *read_item(svn_ra_svn
 
       /* Read in the list items. */
       item->kind = SVN_RA_SVN_LIST;
-      item->u.list = apr_pcalloc(pool, sizeof(*item->u.list));
-
       while (1)
         {
           SVN_ERR(readbuf_getchar_skip_whitespace(conn, pool, &c));
@@ -1327,14 +1325,19 @@ static svn_error_t *read_item(svn_ra_svn
       /* Store the list in ITEM - if not empty (= default). */
       if (count)
         {
-          item->u.list->nelts = count;
+          item->u.list.nelts = count;
 
           /* If we haven't allocated from POOL, yet, do it now. */
           if (items == stack_items)
-            item->u.list->items = apr_pmemdup(pool, items,
-                                              count * sizeof(*items));
+            item->u.list.items = apr_pmemdup(pool, items,
+                                             count * sizeof(*items));
           else
-            item->u.list->items = items;
+            item->u.list.items = items;
+        }
+      else
+        {
+          item->u.list.items = NULL;
+          item->u.list.nelts = 0;
         }
 
       SVN_ERR(readbuf_getchar(conn, pool, &c));
@@ -1508,7 +1511,7 @@ vparse_tuple(const svn_ra_svn__list_t *i
       if (**fmt == '(' && elt->kind == SVN_RA_SVN_LIST)
         {
           (*fmt)++;
-          SVN_ERR(vparse_tuple(elt->u.list, pool, fmt, ap));
+          SVN_ERR(vparse_tuple(&elt->u.list, pool, fmt, ap));
         }
       else if (**fmt == 'c' && elt->kind == SVN_RA_SVN_STRING)
         *va_arg(*ap, const char **) = elt->u.string->data;
@@ -1548,7 +1551,7 @@ vparse_tuple(const svn_ra_svn__list_t *i
             break;
         }
       else if (**fmt == 'l' && elt->kind == SVN_RA_SVN_LIST)
-        *va_arg(*ap, svn_ra_svn__list_t **) = elt->u.list;
+        *va_arg(*ap, svn_ra_svn__list_t **) = &elt->u.list;
       else if (**fmt == ')')
         return SVN_NO_ERROR;
       else
@@ -1629,7 +1632,7 @@ svn_ra_svn__read_tuple(svn_ra_svn_conn_t
     return svn_error_create(SVN_ERR_RA_SVN_MALFORMED_DATA, NULL,
                             _("Malformed network data"));
   va_start(ap, fmt);
-  err = vparse_tuple(item->u.list, pool, &fmt, &ap);
+  err = vparse_tuple(&item->u.list, pool, &fmt, &ap);
   va_end(ap);
   return err;
 }
@@ -1664,7 +1667,7 @@ svn_ra_svn__parse_proplist(const svn_ra_
       if (elt->kind != SVN_RA_SVN_LIST)
         return svn_error_create(SVN_ERR_RA_SVN_MALFORMED_DATA, NULL,
                                 _("Proplist element not a list"));
-      SVN_ERR(svn_ra_svn__parse_tuple(elt->u.list, pool, "ss",
+      SVN_ERR(svn_ra_svn__parse_tuple(&elt->u.list, pool, "ss",
                                       &name, &value));
       apr_hash_set(*props, name->data, name->len, value);
     }
@@ -1713,7 +1716,7 @@ svn_ra_svn__handle_failure_status(const
       if (elt->kind != SVN_RA_SVN_LIST)
         return svn_error_create(SVN_ERR_RA_SVN_MALFORMED_DATA, NULL,
                                 _("Malformed error list"));
-      SVN_ERR(svn_ra_svn__parse_tuple(elt->u.list, subpool, "nccn",
+      SVN_ERR(svn_ra_svn__parse_tuple(&elt->u.list, subpool, "nccn",
                                       &apr_err, &message, &file, &line));
       /* The message field should have been optional, but we can't
          easily change that, so "" means a nonexistent message. */
@@ -2902,7 +2905,7 @@ svn_ra_svn__read_list(const svn_ra_svn__
   svn_ra_svn__item_t *elt  = &SVN_RA_SVN__LIST_ITEM(items, idx);
   CHECK_PROTOCOL_COND(elt->kind == SVN_RA_SVN_LIST);
 
-  *result = elt->u.list;
+  *result = &elt->u.list;
   return SVN_NO_ERROR;
 }
 

Modified: subversion/trunk/subversion/svnserve/serve.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svnserve/serve.c?rev=1701318&r1=1701317&r2=1701318&view=diff
==============================================================================
--- subversion/trunk/subversion/svnserve/serve.c (original)
+++ subversion/trunk/subversion/svnserve/serve.c Fri Sep  4 19:42:42 2015
@@ -1343,12 +1343,12 @@ add_lock_tokens(const svn_ra_svn__list_t
         return svn_error_create(SVN_ERR_RA_SVN_MALFORMED_DATA, NULL,
                                 "Lock tokens aren't a list of lists");
 
-      path_item = &SVN_RA_SVN__LIST_ITEM(item->u.list, 0);
+      path_item = &SVN_RA_SVN__LIST_ITEM(&item->u.list, 0);
       if (path_item->kind != SVN_RA_SVN_STRING)
         return svn_error_create(SVN_ERR_RA_SVN_MALFORMED_DATA, NULL,
                                 "Lock path isn't a string");
 
-      token_item = &SVN_RA_SVN__LIST_ITEM(item->u.list, 1);
+      token_item = &SVN_RA_SVN__LIST_ITEM(&item->u.list, 1);
       if (token_item->kind != SVN_RA_SVN_STRING)
         return svn_error_create(SVN_ERR_RA_SVN_MALFORMED_DATA, NULL,
                                 "Lock token isn't a string");
@@ -1402,8 +1402,8 @@ unlock_paths(const svn_ra_svn__list_t *l
       const char *path, *token, *full_path;
 
       item = &SVN_RA_SVN__LIST_ITEM(lock_tokens, i);
-      path_item = &SVN_RA_SVN__LIST_ITEM(item->u.list, 0);
-      token_item = &SVN_RA_SVN__LIST_ITEM(item->u.list, 1);
+      path_item = &SVN_RA_SVN__LIST_ITEM(&item->u.list, 0);
+      token_item = &SVN_RA_SVN__LIST_ITEM(&item->u.list, 1);
 
       path = path_item->u.string->data;
       full_path = svn_fspath__join(sb->repository->fs_path->data,
@@ -2885,7 +2885,7 @@ lock_many(svn_ra_svn_conn_t *conn,
         return svn_error_create(SVN_ERR_RA_SVN_MALFORMED_DATA, NULL,
                                 "Lock requests should be list of lists");
 
-      SVN_ERR(svn_ra_svn__parse_tuple(item->u.list, subpool, "c(?r)", &path,
+      SVN_ERR(svn_ra_svn__parse_tuple(&item->u.list, subpool, "c(?r)", &path,
                                       &current_rev));
 
       full_path = svn_fspath__join(b->repository->fs_path->data,
@@ -2944,8 +2944,8 @@ lock_many(svn_ra_svn_conn_t *conn,
 
       svn_pool_clear(subpool);
 
-      write_err = svn_ra_svn__parse_tuple(item->u.list, subpool, "c(?r)", &path,
-                                          &current_rev);
+      write_err = svn_ra_svn__parse_tuple(&item->u.list, subpool, "c(?r)",
+                                          &path, &current_rev);
       if (write_err)
         break;
 
@@ -3066,7 +3066,7 @@ unlock_many(svn_ra_svn_conn_t *conn,
         return svn_error_create(SVN_ERR_RA_SVN_MALFORMED_DATA, NULL,
                                 "Unlock request should be a list of lists");
 
-      SVN_ERR(svn_ra_svn__parse_tuple(item->u.list, subpool, "c(?c)", &path,
+      SVN_ERR(svn_ra_svn__parse_tuple(&item->u.list, subpool, "c(?c)", &path,
                                       &token));
       if (!token)
         token = "";
@@ -3124,8 +3124,8 @@ unlock_many(svn_ra_svn_conn_t *conn,
 
       svn_pool_clear(subpool);
 
-      write_err = svn_ra_svn__parse_tuple(item->u.list, subpool, "c(?c)", &path,
-                                          &token);
+      write_err = svn_ra_svn__parse_tuple(&item->u.list, subpool, "c(?c)",
+                                          &path, &token);
       if (write_err)
         break;