Hi devs,
I reworked the patch set according to the feedback I got
here on this list. This is what changed in this last part:
* add rationales to the commentary
* bring svndiff.c changes closer to the original code
* use svn_ctype_* functions in checksum parser
-- Stefan^2.
[[[
Various local optimizations. These opportunities became
visible after significantly optimizing other parts of svn. The
total savings for a 'svn export' is almost 3 percent on top
of the previous.
The largest savings can be attributed to the svndiff.c
changes (~1.5%) and the checksum parser (~1%).
* subversion/include/svn_delta.h
(enum svn_delta_action): ensure that these definitions
will always have these values, even if new definitions
were added
* subversion/libsvn_delta/compose_delta.c
(search_offset_index): use size_t to index memory.
This is mainly a consistency fix but may actually result
in slightly higher performance due to fewer conversions.
(copy_source_ops): dito; optimize a common case
* subversion/libsvn_delta/svndiff.c
(decode_file_offset, decode_size): use slightly more
efficient formulations
(decode_instruction): directly map action codes -
made possible by defining the enum values
* subversion/libsvn_subr/checksum.c
(svn_checksum_parse_hex): eliminate calls to locale-
aware CRT functions. At least with MS compilers,
these are very expensive.
* subversion/libsvn_subr/stream.c
(stream_readline): numbytes is invariant until EOF
patch by stefanfuhrmann < at > alice-dsl.de
]]]
Index: subversion/include/svn_delta.h
===================================================================
--- subversion/include/svn_delta.h (revision 939976)
+++ subversion/include/svn_delta.h (working copy)
@@ -88,6 +88,10 @@
* When svn_txdelta_next_window() returns zero, we are done building
* the target string.
*
+ * Please note that the values assigned in these enumeration must
+ * never be changed. They must match the instruction encoding value.
+ * You are, however, free to add new, non-overlapping definitions.
+ *
* @defgroup svn_delta_txt_delta Text deltas
* @{
*/
@@ -100,7 +104,7 @@
* It must be the case that 0 <= @a offset < @a offset +
* @a length <= size of source view.
*/
- svn_txdelta_source,
+ svn_txdelta_source = 0,
/** Append the @a length bytes at @a offset in the target view, to the
* target.
@@ -119,7 +123,7 @@
* useful in encoding long runs of consecutive characters, long runs
* of CR/LF pairs, etc.
*/
- svn_txdelta_target,
+ svn_txdelta_target = 1,
/** Append the @a length bytes at @a offset in the window's @a new string
* to the target.
@@ -129,7 +133,7 @@
* order with no overlap at the moment; svn_txdelta_to_svndiff()
* depends on this.
*/
- svn_txdelta_new
+ svn_txdelta_new = 2
};
/** A single text delta instruction. */
Index: subversion/libsvn_delta/compose_delta.c
===================================================================
--- subversion/libsvn_delta/compose_delta.c (revision 939976)
+++ subversion/libsvn_delta/compose_delta.c (working copy)
@@ -165,10 +165,10 @@
as hint because most lookups come as a sequence of decreasing values
for OFFSET and they concentrate on the lower end of the array. */
-static int
-search_offset_index(const offset_index_t *ndx, apr_size_t offset, int hint)
+static apr_size_t
+search_offset_index(const offset_index_t *ndx, apr_size_t offset, apr_size_t
hint)
{
- int lo, hi, op;
+ apr_size_t lo, hi, op;
assert(offset < ndx->offs[ndx->length]);
@@ -635,13 +635,13 @@
static void
copy_source_ops(apr_size_t offset, apr_size_t limit,
apr_size_t target_offset,
- int hint,
+ apr_size_t hint,
svn_txdelta__ops_baton_t *build_baton,
const svn_txdelta_window_t *window,
const offset_index_t *ndx,
apr_pool_t *pool)
{
- int op_ndx = search_offset_index(ndx, offset, hint);
+ apr_size_t op_ndx = search_offset_index(ndx, offset, hint);
for (;; ++op_ndx)
{
const svn_txdelta_op_t *const op = &window->ops[op_ndx];
@@ -694,7 +694,15 @@
The idea here is to transpose the pattern, then generate
another overlapping copy. */
const apr_size_t ptn_length = off[0] - op->offset;
- const apr_size_t ptn_overlap = fix_offset % ptn_length;
+
+ /* As it turns out, ptn_length is often less than 3.
+ We can use a more efficient modulo is that frequent case.
+ Please note that this optimization is only valid due to
+ the special case actually being the major case. */
+ const apr_size_t ptn_overlap = ptn_length > 2
+ ? fix_offset % ptn_length
+ : fix_offset & (ptn_length-1);
+
apr_size_t fix_off = fix_offset;
apr_size_t tgt_off = target_offset;
assert(ptn_length > ptn_overlap);
Index: subversion/libsvn_delta/svndiff.c
===================================================================
--- subversion/libsvn_delta/svndiff.c (revision 939976)
+++ subversion/libsvn_delta/svndiff.c (working copy)
@@ -359,16 +359,27 @@
const unsigned char *p,
const unsigned char *end)
{
+ svn_filesize_t temp = 0;
+
if (p + MAX_ENCODED_INT_LEN < end)
end = p + MAX_ENCODED_INT_LEN;
/* Decode bytes until we're done. */
- *val = 0;
while (p < end)
{
- *val = (*val << 7) | (*p & 0x7f);
- if (((*p++ >> 7) & 0x1) == 0)
+ /* Don't use svn_filesize_t here, because this might be 64 bits
+ * on 32 bit targets. Optimizing compilers may or may not be
+ * able to reduce that to the effective code below. */
+ unsigned int c = *p++;
+
+ temp = (temp << 7) | (c & 0x7f);
+ if (c < 0x80)
+ {
+ *val = temp;
return p;
+ }
}
+
+ *val = temp;
return NULL;
}
@@ -379,16 +390,24 @@
const unsigned char *p,
const unsigned char *end)
{
+ apr_size_t temp = 0;
+
if (p + MAX_ENCODED_INT_LEN < end)
end = p + MAX_ENCODED_INT_LEN;
/* Decode bytes until we're done. */
- *val = 0;
while (p < end)
{
- *val = (*val << 7) | (*p & 0x7f);
- if (((*p++ >> 7) & 0x1) == 0)
+ apr_size_t c = *p++;
+
+ temp = (temp << 7) | (c & 0x7f);
+ if (c < 0x80)
+ {
+ *val = temp;
return p;
+ }
}
+
+ *val = temp;
return NULL;
}
@@ -453,27 +472,33 @@
const unsigned char *p,
const unsigned char *end)
{
+ apr_size_t c;
+ apr_size_t action;
+
if (p == end)
return NULL;
+ /* We need this more than once */
+ c = *p++;
+
/* Decode the instruction selector. */
- switch ((*p >> 6) & 0x3)
- {
- case 0x0: op->action_code = svn_txdelta_source; break;
- case 0x1: op->action_code = svn_txdelta_target; break;
- case 0x2: op->action_code = svn_txdelta_new; break;
- case 0x3: return NULL;
- }
+ action = (c >> 6) & 0x3;
+ if (action >= 0x3)
+ return NULL;
+ /* This relies on enum svn_delta_action values to match and
+ never to be redefined. */
+ op->action_code = (enum svn_delta_action)(action);
+
/* Decode the length and offset. */
- op->length = *p++ & 0x3f;
+ op->length = c & 0x3f;
if (op->length == 0)
{
p = decode_size(&op->length, p, end);
if (p == NULL)
return NULL;
}
- if (op->action_code != svn_txdelta_new)
+ if (action != svn_txdelta_new)
{
p = decode_size(&op->offset, p, end);
if (p == NULL)
Index: subversion/libsvn_subr/checksum.c
===================================================================
--- subversion/libsvn_subr/checksum.c (revision 939976)
+++ subversion/libsvn_subr/checksum.c (working copy)
@@ -29,6 +29,7 @@
#include "svn_checksum.h"
#include "svn_error.h"
+#include "svn_ctype.h"
#include "sha1.h"
#include "md5.h"
@@ -206,12 +207,15 @@
for (i = 0; i < len; i++)
{
- if ((! isxdigit(hex[i * 2])) || (! isxdigit(hex[i * 2 + 1])))
+ if ((! svn_ctype_isxdigit(hex[i * 2])) ||
+ (! svn_ctype_isxdigit(hex[i * 2 + 1])))
return svn_error_create(SVN_ERR_BAD_CHECKSUM_PARSE, NULL, NULL);
((unsigned char *)(*checksum)->digest)[i] =
- (( isalpha(hex[i*2]) ? hex[i*2] - 'a' + 10 : hex[i*2] - '0') << 4) |
- ( isalpha(hex[i*2+1]) ? hex[i*2+1] - 'a' + 10 : hex[i*2+1] - '0');
+ (( svn_ctype_isalpha(hex[i*2]) ? hex[i*2] - 'a' + 10
+ : hex[i*2] - '0') << 4) |
+ ( svn_ctype_isalpha(hex[i*2+1]) ? hex[i*2+1] - 'a' + 10
+ : hex[i*2+1] - '0');
is_zeros |= (*checksum)->digest[i];
}
Index: subversion/libsvn_subr/stream.c
===================================================================
--- subversion/libsvn_subr/stream.c (revision 937673)
+++ subversion/libsvn_subr/stream.c (working copy)
@@ -355,9 +355,9 @@
/* Read into STR up to and including the next EOL sequence. */
match = eol_str;
+ numbytes = 1;
while (*match)
{
- numbytes = 1;
SVN_ERR(svn_stream_read(stream, &c, &numbytes));
if (numbytes != 1)
{
