subversion-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From stef...@apache.org
Subject svn commit: r1325899 - in /subversion/trunk/subversion/libsvn_ra_svn: marshal.c ra_svn.h
Date Fri, 13 Apr 2012 18:36:46 GMT
Author: stefan2
Date: Fri Apr 13 18:36:46 2012
New Revision: 1325899

URL: http://svn.apache.org/viewvc?rev=1325899&view=rev
Log:
Various performance improvements to the ra_svn marshaller.
The total effect is roughly a duplication in throughput.

* subversion/libsvn_ra_svn/ra_svn.h
  (SVN_RA_SVN__READBUF_SIZE, SVN_RA_SVN__WRITEBUF_SIZE):
   increase buffer size to 16kByte
  (svn_ra_svn_conn_st): use apr_size_t for sizes
* subversion/libsvn_ra_svn/marshal.c
  (writebuf_flush): use apr_size_t for sizes
  (writebuf_printf): drop
  (writebuf_write_short_string, writebuf_writechar, write_number):
   new, specialized write functions
  (svn_ra_svn_write_number, svn_ra_svn_write_string,
   svn_ra_svn_write_cstring, svn_ra_svn_write_word,
   svn_ra_svn_start_list, svn_ra_svn_end_list): use new functions
  (vwrite_tuple): reorder cases to favor frequent control chars
  (read_item): eliminate expensive 64 bit division from hot path

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

Modified: subversion/trunk/subversion/libsvn_ra_svn/marshal.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_svn/marshal.c?rev=1325899&r1=1325898&r2=1325899&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_svn/marshal.c (original)
+++ subversion/trunk/subversion/libsvn_ra_svn/marshal.c Fri Apr 13 18:36:46 2012
@@ -231,7 +231,7 @@ static svn_error_t *writebuf_output(svn_
 /* Write data from the write buffer out to the socket. */
 static svn_error_t *writebuf_flush(svn_ra_svn_conn_t *conn, apr_pool_t *pool)
 {
-  int write_pos = conn->write_pos;
+  apr_size_t write_pos = conn->write_pos;
 
   /* Clear conn->write_pos first in case the block handler does a read. */
   conn->write_pos = 0;
@@ -258,19 +258,36 @@ static svn_error_t *writebuf_write(svn_r
   return SVN_NO_ERROR;
 }
 
-static svn_error_t *writebuf_printf(svn_ra_svn_conn_t *conn, apr_pool_t *pool,
-                                    const char *fmt, ...)
-  __attribute__ ((format(printf, 3, 4)));
-static svn_error_t *writebuf_printf(svn_ra_svn_conn_t *conn, apr_pool_t *pool,
-                                    const char *fmt, ...)
+static svn_error_t *
+writebuf_write_short_string(svn_ra_svn_conn_t *conn, apr_pool_t *pool,
+                            const char *data, apr_size_t len)
+{
+  apr_size_t left = sizeof(conn->write_buf) - conn->write_pos;
+  if (len <= left)
+  {
+    memcpy(conn->write_buf + conn->write_pos, data, len);
+    conn->write_pos += len;
+    return SVN_NO_ERROR;
+  }
+  else
+    return writebuf_write(conn, pool, data, len);
+}
+
+static APR_INLINE svn_error_t *
+writebuf_writechar(svn_ra_svn_conn_t *conn, apr_pool_t *pool, char data)
 {
-  va_list ap;
-  char *str;
+  if (conn->write_pos < sizeof(conn->write_buf))
+  {
+    conn->write_buf[conn->write_pos] = data;
+    conn->write_pos++;
 
-  va_start(ap, fmt);
-  str = apr_pvsprintf(pool, fmt, ap);
-  va_end(ap);
-  return writebuf_write(conn, pool, str, strlen(str));
+    return SVN_NO_ERROR;
+  }
+  else
+  {
+    char temp = data;
+    return writebuf_write(conn, pool, &temp, 1);
+  }
 }
 
 /* --- READ BUFFER MANAGEMENT --- */
@@ -417,31 +434,69 @@ static svn_error_t *readbuf_skip_leading
 
 /* --- WRITING DATA ITEMS --- */
 
+static svn_error_t *write_number(svn_ra_svn_conn_t *conn, apr_pool_t *pool,
+                                 apr_uint64_t number, char follow)
+{
+  apr_size_t written;
+
+  if (conn->write_pos + SVN_INT64_BUFFER_SIZE >= sizeof(conn->write_buf))
+    SVN_ERR(writebuf_flush(conn, pool));
+
+  written = svn__ui64toa(conn->write_buf + conn->write_pos, number);
+  conn->write_buf[conn->write_pos + written] = follow;
+  conn->write_pos += written + 1;
+
+  return SVN_NO_ERROR;
+}
+
 svn_error_t *svn_ra_svn_write_number(svn_ra_svn_conn_t *conn, apr_pool_t *pool,
                                      apr_uint64_t number)
 {
-  return writebuf_printf(conn, pool, "%" APR_UINT64_T_FMT " ", number);
+  return write_number(conn, pool, number, ' ');
 }
 
 svn_error_t *svn_ra_svn_write_string(svn_ra_svn_conn_t *conn, apr_pool_t *pool,
                                      const svn_string_t *str)
 {
-  SVN_ERR(writebuf_printf(conn, pool, "%" APR_SIZE_T_FMT ":", str->len));
+  if (str->len < 10)
+    {
+      SVN_ERR(writebuf_writechar(conn, pool, (char)str->len + '0'));
+      SVN_ERR(writebuf_writechar(conn, pool, ':'));
+    }
+  else
+    write_number(conn, pool, str->len, ':');
+
   SVN_ERR(writebuf_write(conn, pool, str->data, str->len));
-  SVN_ERR(writebuf_write(conn, pool, " ", 1));
+  SVN_ERR(writebuf_writechar(conn, pool, ' '));
   return SVN_NO_ERROR;
 }
 
 svn_error_t *svn_ra_svn_write_cstring(svn_ra_svn_conn_t *conn,
                                       apr_pool_t *pool, const char *s)
 {
-  return writebuf_printf(conn, pool, "%" APR_SIZE_T_FMT ":%s ", strlen(s), s);
+  apr_size_t len = strlen(s);
+
+  if (len < 10)
+    {
+      SVN_ERR(writebuf_writechar(conn, pool, (char)len + '0'));
+      SVN_ERR(writebuf_writechar(conn, pool, ':'));
+    }
+  else
+    write_number(conn, pool, len, ':');
+
+  SVN_ERR(writebuf_write(conn, pool, s, len));
+  SVN_ERR(writebuf_writechar(conn, pool, ' '));
+
+  return SVN_NO_ERROR;
 }
 
 svn_error_t *svn_ra_svn_write_word(svn_ra_svn_conn_t *conn, apr_pool_t *pool,
                                    const char *word)
 {
-  return writebuf_printf(conn, pool, "%s ", word);
+  SVN_ERR(writebuf_write_short_string(conn, pool, word, strlen(word)));
+  SVN_ERR(writebuf_writechar(conn, pool, ' '));
+
+  return SVN_NO_ERROR;
 }
 
 svn_error_t *svn_ra_svn_write_proplist(svn_ra_svn_conn_t *conn,
@@ -474,11 +529,27 @@ svn_error_t *svn_ra_svn_write_proplist(s
 
 svn_error_t *svn_ra_svn_start_list(svn_ra_svn_conn_t *conn, apr_pool_t *pool)
 {
+  if (conn->write_pos + 2 <= sizeof(conn->write_buf))
+    {
+      conn->write_buf[conn->write_pos] = '(';
+      conn->write_buf[conn->write_pos+1] = ' ';
+      conn->write_pos += 2;
+      return SVN_NO_ERROR;
+    }
+
   return writebuf_write(conn, pool, "( ", 2);
 }
 
 svn_error_t *svn_ra_svn_end_list(svn_ra_svn_conn_t *conn, apr_pool_t *pool)
 {
+  if (conn->write_pos + 2 <= sizeof(conn->write_buf))
+  {
+    conn->write_buf[conn->write_pos] = ')';
+    conn->write_buf[conn->write_pos+1] = ' ';
+    conn->write_pos += 2;
+    return SVN_NO_ERROR;
+  }
+
   return writebuf_write(conn, pool, ") ", 2);
 }
 
@@ -503,50 +574,54 @@ static svn_error_t *vwrite_tuple(svn_ra_
     SVN_ERR(svn_ra_svn_start_list(conn, pool));
   for (; *fmt; fmt++)
     {
-      if (*fmt == 'n' && !opt)
-        SVN_ERR(svn_ra_svn_write_number(conn, pool, va_arg(ap, apr_uint64_t)));
-      else if (*fmt == 'r')
+      if (*fmt == 'c')
         {
-          rev = va_arg(ap, svn_revnum_t);
-          SVN_ERR_ASSERT(opt || SVN_IS_VALID_REVNUM(rev));
-          if (SVN_IS_VALID_REVNUM(rev))
-            SVN_ERR(svn_ra_svn_write_number(conn, pool, rev));
+          cstr = va_arg(ap, const char *);
+          if (cstr)
+            SVN_ERR(svn_ra_svn_write_cstring(conn, pool, cstr));
+          else
+            SVN_ERR_ASSERT(opt);
         }
       else if (*fmt == 's')
         {
           str = va_arg(ap, const svn_string_t *);
-          SVN_ERR_ASSERT(opt || str);
           if (str)
             SVN_ERR(svn_ra_svn_write_string(conn, pool, str));
+          else
+            SVN_ERR_ASSERT(opt);
         }
-      else if (*fmt == 'c')
+      else if (*fmt == '(' && !opt)
+        SVN_ERR(svn_ra_svn_start_list(conn, pool));
+      else if (*fmt == ')')
         {
-          cstr = va_arg(ap, const char *);
-          SVN_ERR_ASSERT(opt || cstr);
-          if (cstr)
-            SVN_ERR(svn_ra_svn_write_cstring(conn, pool, cstr));
+          SVN_ERR(svn_ra_svn_end_list(conn, pool));
+          opt = FALSE;
         }
+      else if (*fmt == '?')
+        opt = TRUE;
       else if (*fmt == 'w')
         {
           cstr = va_arg(ap, const char *);
-          SVN_ERR_ASSERT(opt || cstr);
           if (cstr)
             SVN_ERR(svn_ra_svn_write_word(conn, pool, cstr));
+          else
+            SVN_ERR_ASSERT(opt);
         }
+      else if (*fmt == 'r')
+        {
+          rev = va_arg(ap, svn_revnum_t);
+          if (SVN_IS_VALID_REVNUM(rev))
+            SVN_ERR(svn_ra_svn_write_number(conn, pool, rev));
+          else
+            SVN_ERR_ASSERT(opt);
+        }
+      else if (*fmt == 'n' && !opt)
+        SVN_ERR(svn_ra_svn_write_number(conn, pool, va_arg(ap, apr_uint64_t)));
       else if (*fmt == 'b' && !opt)
         {
           cstr = va_arg(ap, svn_boolean_t) ? "true" : "false";
           SVN_ERR(svn_ra_svn_write_word(conn, pool, cstr));
         }
-      else if (*fmt == '?')
-        opt = TRUE;
-      else if (*fmt == '(' && !opt)
-        SVN_ERR(svn_ra_svn_start_list(conn, pool));
-      else if (*fmt == ')')
-        {
-          SVN_ERR(svn_ra_svn_end_list(conn, pool));
-          opt = FALSE;
-        }
       else if (*fmt == '!' && !*(fmt + 1))
         return SVN_NO_ERROR;
       else
@@ -668,7 +743,8 @@ static svn_error_t *read_item(svn_ra_svn
           if (!svn_ctype_isdigit(c))
             break;
           val = val * 10 + (c - '0');
-          if ((val / 10) != prev_val) /* val wrapped past maximum value */
+          /* val wrapped past maximum value? */
+          if (prev_val >= (APR_UINT64_MAX / 10) && (val / 10) != prev_val)
             return svn_error_create(SVN_ERR_RA_SVN_MALFORMED_DATA, NULL,
                                     _("Number is larger than maximum"));
         }

Modified: subversion/trunk/subversion/libsvn_ra_svn/ra_svn.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_svn/ra_svn.h?rev=1325899&r1=1325898&r2=1325899&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_svn/ra_svn.h (original)
+++ subversion/trunk/subversion/libsvn_ra_svn/ra_svn.h Fri Apr 13 18:36:46 2012
@@ -57,8 +57,8 @@ typedef svn_error_t *(*ra_svn_block_hand
                                                void *baton);
 
 /* The size of our per-connection read and write buffers. */
-#define SVN_RA_SVN__READBUF_SIZE 4096
-#define SVN_RA_SVN__WRITEBUF_SIZE 4096
+#define SVN_RA_SVN__READBUF_SIZE 4*4096
+#define SVN_RA_SVN__WRITEBUF_SIZE 4*4096
 
 /* Create forward reference */
 typedef struct svn_ra_svn__session_baton_t svn_ra_svn__session_baton_t;
@@ -79,7 +79,7 @@ struct svn_ra_svn_conn_st {
   char *read_ptr;
   char *read_end;
   char write_buf[SVN_RA_SVN__WRITEBUF_SIZE];
-  int write_pos;
+  apr_size_t write_pos;
   const char *uuid;
   const char *repos_root;
   ra_svn_block_handler_t block_handler;



Mime
View raw message