> New Revision: 997203
>
> URL: http://svn.apache.org/viewvc?rev=997203&view=rev
> Log:
> Merge r985037, r985046, r995507 and r995603 from the performance branch.
>
> These changes introduce the svn_stringbuf_appendbyte() function, which has
> significantly less overhead than svn_stringbuf_appendbytes(), and can be
> used in a number of places within our codebase.
Hi Stefan2.
I have been wondering about the actual benefit of such tightly
hand-optimized code. Today I got around to giving it a quick spin.
In my first test, it made no difference whatsoever, because the
optimized code is never executed. The recent merge r1002898 of r1001413
from the performance branch introduced a bug:
- if (str->blocksize > old_len + 1)
+ if (str->blocksize < old_len + 1)
which totally disables the optimized code path.
Fixed in r1005437.
That solved, a loop doing a million simple appendbyte calls runs more
than twice as fast as calling appendbytes(..., 1). That's fantastic.
But what about that hand-optimization? I wrote a simple version of the
function, and called it 'appendbyte0':
svn_stringbuf_appendbyte0(svn_stringbuf_t *str, char byte)
{
if (str->blocksize > str->len + 1)
{
str->data[str->len++] = byte;
str->data[str->len] = '\0';
}
else
svn_stringbuf_appendbytes(str, &byte, 1);
}
Here are the results (see full patch attached):
Times: appendbytes appendbyte0 appendbyte (in ms)
run: 89 31 34
run: 88 30 35
run: 88 31 34
run: 88 30 34
run: 88 31 34
min: 88 30 34
This tells me that the hand-optimization is actually harmful and the
compiler does a 10% better job by itself.
Have I made a mistake?
What are the results for your system?
(I'm using GCC 4.4.1 on an Intel Centrino laptop CPU.)
- Julian
A comparative performance test of svn_stringbuf_appendbyte().
* subversion/libsvn_subr/svn_string.c
(svn_stringbuf_appendbyte0): New function, for comparative testing.
* subversion/tests/libsvn_subr/string-test.c
(test24): New test, measuring and printing a speed comparison.
(test_funcs): Add it.
--This line, and those below, will be ignored--
Index: subversion/libsvn_subr/svn_string.c
===================================================================
--- subversion/libsvn_subr/svn_string.c (revision 1005437)
+++ subversion/libsvn_subr/svn_string.c (working copy)
@@ -392,6 +392,29 @@ svn_stringbuf_ensure(svn_stringbuf_t *st
}
+/* Unoptimized version of svn_stringbuf_appendbyte(), for comparison. */
+void
+svn_stringbuf_appendbyte0(svn_stringbuf_t *str, char byte)
+{
+ /* In most cases, there will be pre-allocated memory left
+ * to just write the new byte at the end of the used section
+ * and terminate the string properly.
+ */
+ if (str->blocksize > str->len + 1)
+ {
+ str->data[str->len++] = byte;
+ str->data[str->len] = '\0';
+ }
+ else
+ {
+ /* we need to re-allocate the string buffer
+ * -> let the more generic implementation take care of that part
+ */
+ svn_stringbuf_appendbytes(str, &byte, 1);
+ }
+}
+
+
/* WARNING - Optimized code ahead!
* This function has been hand-tuned for performance. Please read
* the comments below before modifying the code.
Index: subversion/tests/libsvn_subr/string-test.c
===================================================================
--- subversion/tests/libsvn_subr/string-test.c (revision 1003481)
+++ subversion/tests/libsvn_subr/string-test.c (working copy)
@@ -511,6 +511,50 @@ test23(apr_pool_t *pool)
return test_stringbuf_unequal("abc", "abb", pool);
}
+void
+svn_stringbuf_appendbyte0(svn_stringbuf_t *str, char byte);
+
+static svn_error_t *
+test24(apr_pool_t *pool)
+{
+ apr_time_t start;
+ int t1, t2, t3;
+ int min_t1 = 999999, min_t2 = 999999, min_t3 = 999999;
+ char byte = 'X';
+ int N = 2000000, TESTS = 5;
+ int i, j;
+
+ printf("Times: appendbytes appendbyte0 appendbyte (in ms)\n");
+ for (j = 0; j < TESTS; j++)
+ {
+ a = svn_stringbuf_create("", pool);
+ start = apr_time_now();
+ for (i = 0; i < N; i++)
+ svn_stringbuf_appendbytes(a, &byte, 1);
+ t1 = (int)(apr_time_now() - start) / 1000;
+ if (t1 < min_t1) min_t1 = t1;
+
+ a = svn_stringbuf_create("", pool);
+ start = apr_time_now();
+ for (i = 0; i < N; i++)
+ svn_stringbuf_appendbyte0(a, byte);
+ t2 = (int)(apr_time_now() - start) / 1000;
+ if (t2 < min_t2) min_t2 = t2;
+
+ a = svn_stringbuf_create("", pool);
+ start = apr_time_now();
+ for (i = 0; i < N; i++)
+ svn_stringbuf_appendbyte(a, byte);
+ t3 = (int)(apr_time_now() - start) / 1000;
+ if (t3 < min_t3) min_t3 = t3;
+
+ printf("run: %10d %10d %10d\n", t1, t2, t3);
+ }
+ printf("min: %10d %10d %10d\n", min_t1, min_t2, min_t3);
+
+ return SVN_NO_ERROR;
+}
+
/*
====================================================================
If you add a new test to this file, update this array.
@@ -568,5 +612,7 @@ struct svn_test_descriptor_t test_funcs[
"compare stringbufs; different lengths"),
SVN_TEST_PASS2(test23,
"compare stringbufs; same length, different content"),
+ SVN_TEST_PASS2(test24,
+ "benchmark svn_stringbuf_appendbyte"),
SVN_TEST_NULL
};
