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 2010/08/15 02:20:35 UTC
svn commit: r985606 -
/subversion/branches/performance/subversion/libsvn_ra_svn/marshal.c
Author: stefan2
Date: Sun Aug 15 00:20:34 2010
New Revision: 985606
URL: http://svn.apache.org/viewvc?rev=985606&view=rev
Log:
Increase the RA_SVN throughput by reducing the overhead in read_item for
packing received data into various structures.
* subversion/libsvn_ra_svn/marshal.c
(readbuf_getchar): suggest inlining to the compiler
(read_long_string): new fallback function containing the data receiving part
of the former read_string
(read_string): use special code that eliminates re-alloc operations for non-huge strings
(read_item): ensure reasonable container sizes to minimize the number of re-allocs
Modified:
subversion/branches/performance/subversion/libsvn_ra_svn/marshal.c
Modified: subversion/branches/performance/subversion/libsvn_ra_svn/marshal.c
URL: http://svn.apache.org/viewvc/subversion/branches/performance/subversion/libsvn_ra_svn/marshal.c?rev=985606&r1=985605&r2=985606&view=diff
==============================================================================
--- subversion/branches/performance/subversion/libsvn_ra_svn/marshal.c (original)
+++ subversion/branches/performance/subversion/libsvn_ra_svn/marshal.c Sun Aug 15 00:20:34 2010
@@ -314,8 +314,8 @@ static svn_error_t *readbuf_fill(svn_ra_
return SVN_NO_ERROR;
}
-static svn_error_t *readbuf_getchar(svn_ra_svn_conn_t *conn, apr_pool_t *pool,
- char *result)
+static APR_INLINE svn_error_t *
+readbuf_getchar(svn_ra_svn_conn_t *conn, apr_pool_t *pool, char *result)
{
if (conn->read_ptr == conn->read_end)
SVN_ERR(readbuf_fill(conn, pool));
@@ -558,12 +558,12 @@ svn_error_t *svn_ra_svn_write_tuple(svn_
/* Read LEN bytes from CONN into already-allocated structure ITEM.
* Afterwards, *ITEM is of type 'SVN_RA_SVN_STRING', and its string
* data is allocated in POOL. */
-static svn_error_t *read_string(svn_ra_svn_conn_t *conn, apr_pool_t *pool,
- svn_ra_svn_item_t *item, apr_uint64_t len)
+static svn_error_t *
+read_long_string(svn_ra_svn_conn_t *conn, apr_pool_t *pool,
+ svn_stringbuf_t *stringbuf, apr_uint64_t len)
{
char readbuf[4096];
apr_size_t readbuf_len;
- svn_stringbuf_t *stringbuf = svn_stringbuf_create("", pool);
/* We can't store strings longer than the maximum size of apr_size_t,
* so check for wrapping */
@@ -583,6 +583,50 @@ static svn_error_t *read_string(svn_ra_s
len -= readbuf_len;
}
+ return SVN_NO_ERROR;
+}
+
+/* Read LEN bytes from CONN into already-allocated structure ITEM.
+ * Afterwards, *ITEM is of type 'SVN_RA_SVN_STRING', and its string
+ * data is allocated in POOL. */
+static svn_error_t *read_string(svn_ra_svn_conn_t *conn, apr_pool_t *pool,
+ svn_ra_svn_item_t *item, apr_uint64_t len)
+{
+ svn_stringbuf_t *stringbuf;
+ if (len > 0x100000)
+ {
+ /* This string might take a large amount of memory. Don't allocate
+ * the whole buffer at once, so to prevent OOM issues by corrupted
+ * network data.
+ */
+ stringbuf = svn_stringbuf_create("", pool);
+ SVN_ERR(read_long_string(conn, pool, stringbuf, len));
+ }
+ else
+ {
+ /* This is a reasonably sized string. So, provide a buffer large
+ * enough to prevent re-allocation as long as the data transmission
+ * is not flawed.
+ */
+ stringbuf = svn_stringbuf_create_ensure(len, pool);
+
+ /* Read the string data directly into the string structure.
+ * Do it iteratively, if necessary.
+ */
+ while (len)
+ {
+ apr_size_t readbuf_len = (apr_size_t)len;
+ char *dest = stringbuf->data + stringbuf->len;
+ SVN_ERR(readbuf_read(conn, pool, dest, readbuf_len));
+
+ stringbuf->len += readbuf_len;
+ stringbuf->data[stringbuf->len] = '\0';
+ len -= readbuf_len;
+ }
+ }
+
+ /* Return the string properly wrapped into an RA_SVN item.
+ */
item->kind = SVN_RA_SVN_STRING;
item->u.string = apr_palloc(pool, sizeof(*item->u.string));
item->u.string->data = stringbuf->data;
@@ -643,7 +687,8 @@ static svn_error_t *read_item(svn_ra_svn
else if (apr_isalpha(c))
{
/* It's a word. */
- str = svn_stringbuf_ncreate(&c, 1, pool);
+ str = svn_stringbuf_create_ensure(16, pool);
+ svn_stringbuf_appendbyte(str, c);
while (1)
{
SVN_ERR(readbuf_getchar(conn, pool, &c));
@@ -658,7 +703,7 @@ static svn_error_t *read_item(svn_ra_svn
{
/* Read in the list items. */
item->kind = SVN_RA_SVN_LIST;
- item->u.list = apr_array_make(pool, 0, sizeof(svn_ra_svn_item_t));
+ item->u.list = apr_array_make(pool, 4, sizeof(svn_ra_svn_item_t));
while (1)
{
SVN_ERR(readbuf_getchar_skip_whitespace(conn, pool, &c));
Re: svn commit: r985606 - /subversion/branches/performance/subversion/libsvn_ra_svn/marshal.c
Posted by "Hyrum K. Wright" <hy...@mail.utexas.edu>.
On Sat, Aug 14, 2010 at 7:20 PM, <st...@apache.org> wrote:
> Author: stefan2
> Date: Sun Aug 15 00:20:34 2010
> New Revision: 985606
>
> URL: http://svn.apache.org/viewvc?rev=985606&view=rev
> Log:
> Increase the RA_SVN throughput by reducing the overhead in read_item for
> packing received data into various structures.
>
> * subversion/libsvn_ra_svn/marshal.c
> (readbuf_getchar): suggest inlining to the compiler
> (read_long_string): new fallback function containing the data receiving part
> of the former read_string
> (read_string): use special code that eliminates re-alloc operations for non-huge strings
> (read_item): ensure reasonable container sizes to minimize the number of re-allocs
>
> Modified:
> subversion/branches/performance/subversion/libsvn_ra_svn/marshal.c
>
> Modified: subversion/branches/performance/subversion/libsvn_ra_svn/marshal.c
> URL: http://svn.apache.org/viewvc/subversion/branches/performance/subversion/libsvn_ra_svn/marshal.c?rev=985606&r1=985605&r2=985606&view=diff
> ==============================================================================
> --- subversion/branches/performance/subversion/libsvn_ra_svn/marshal.c (original)
> +++ subversion/branches/performance/subversion/libsvn_ra_svn/marshal.c Sun Aug 15 00:20:34 2010
> @@ -314,8 +314,8 @@ static svn_error_t *readbuf_fill(svn_ra_
> return SVN_NO_ERROR;
> }
>
> -static svn_error_t *readbuf_getchar(svn_ra_svn_conn_t *conn, apr_pool_t *pool,
> - char *result)
> +static APR_INLINE svn_error_t *
> +readbuf_getchar(svn_ra_svn_conn_t *conn, apr_pool_t *pool, char *result)
> {
> if (conn->read_ptr == conn->read_end)
> SVN_ERR(readbuf_fill(conn, pool));
> @@ -558,12 +558,12 @@ svn_error_t *svn_ra_svn_write_tuple(svn_
> /* Read LEN bytes from CONN into already-allocated structure ITEM.
> * Afterwards, *ITEM is of type 'SVN_RA_SVN_STRING', and its string
> * data is allocated in POOL. */
Docstring now appears out-of-sync with the function parameter list.
> -static svn_error_t *read_string(svn_ra_svn_conn_t *conn, apr_pool_t *pool,
> - svn_ra_svn_item_t *item, apr_uint64_t len)
> +static svn_error_t *
> +read_long_string(svn_ra_svn_conn_t *conn, apr_pool_t *pool,
> + svn_stringbuf_t *stringbuf, apr_uint64_t len)
> {
> char readbuf[4096];
> apr_size_t readbuf_len;
> - svn_stringbuf_t *stringbuf = svn_stringbuf_create("", pool);
>
> /* We can't store strings longer than the maximum size of apr_size_t,
> * so check for wrapping */
> @@ -583,6 +583,50 @@ static svn_error_t *read_string(svn_ra_s
> len -= readbuf_len;
> }
>
> + return SVN_NO_ERROR;
> +}
> +
> +/* Read LEN bytes from CONN into already-allocated structure ITEM.
> + * Afterwards, *ITEM is of type 'SVN_RA_SVN_STRING', and its string
> + * data is allocated in POOL. */
> +static svn_error_t *read_string(svn_ra_svn_conn_t *conn, apr_pool_t *pool,
> + svn_ra_svn_item_t *item, apr_uint64_t len)
> +{
> + svn_stringbuf_t *stringbuf;
> + if (len > 0x100000)
Magic number here. Was this value chosen arbitrarily, or via some
calculation? In either case, might be better to put it into a macro
and then use that.
> + {
> + /* This string might take a large amount of memory. Don't allocate
> + * the whole buffer at once, so to prevent OOM issues by corrupted
> + * network data.
> + */
> + stringbuf = svn_stringbuf_create("", pool);
> + SVN_ERR(read_long_string(conn, pool, stringbuf, len));
> + }
> + else
> + {
> + /* This is a reasonably sized string. So, provide a buffer large
> + * enough to prevent re-allocation as long as the data transmission
> + * is not flawed.
> + */
> + stringbuf = svn_stringbuf_create_ensure(len, pool);
> +
> + /* Read the string data directly into the string structure.
> + * Do it iteratively, if necessary.
> + */
> + while (len)
> + {
> + apr_size_t readbuf_len = (apr_size_t)len;
> + char *dest = stringbuf->data + stringbuf->len;
> + SVN_ERR(readbuf_read(conn, pool, dest, readbuf_len));
> +
> + stringbuf->len += readbuf_len;
> + stringbuf->data[stringbuf->len] = '\0';
> + len -= readbuf_len;
> + }
> + }
> +
> + /* Return the string properly wrapped into an RA_SVN item.
> + */
> item->kind = SVN_RA_SVN_STRING;
> item->u.string = apr_palloc(pool, sizeof(*item->u.string));
> item->u.string->data = stringbuf->data;
> @@ -643,7 +687,8 @@ static svn_error_t *read_item(svn_ra_svn
> else if (apr_isalpha(c))
> {
> /* It's a word. */
> - str = svn_stringbuf_ncreate(&c, 1, pool);
> + str = svn_stringbuf_create_ensure(16, pool);
> + svn_stringbuf_appendbyte(str, c);
> while (1)
> {
> SVN_ERR(readbuf_getchar(conn, pool, &c));
> @@ -658,7 +703,7 @@ static svn_error_t *read_item(svn_ra_svn
> {
> /* Read in the list items. */
> item->kind = SVN_RA_SVN_LIST;
> - item->u.list = apr_array_make(pool, 0, sizeof(svn_ra_svn_item_t));
> + item->u.list = apr_array_make(pool, 4, sizeof(svn_ra_svn_item_t));
> while (1)
> {
> SVN_ERR(readbuf_getchar_skip_whitespace(conn, pool, &c));
>
>
>