Hi devs,
I reworked the patch set according to the feedback I got
here on this list. This is what changed in the first part:
* split original part 1/2 into 1/3 (append char) and 2/3
(apply text delta).
* introduced new svn_stringbuf_appendbyte function
and undid changes to svn_stringbuf_appendbytes
* replace all calls to appendbytes(.., 1) with appendbyte
throughout SVN
-- Stefan^2.
[[[
Introduce a lightweight variant of svn_stringbuf_appendbytes that
adds single bytes to a string. Not only is this a more appropriate
way to add chars, i.e. not treating them as arrays of length 1.
But it also can save measurable amounts of runtime. For svn export,
it saves between .5 to 1 percent total runtime due to the now more
stream_readline code.
Replace calls to svn_stringbuf_appendbytes (..., 1) with calls to
the new svn_stringbuf_appendbyte. Do this in test code as well
to get adequate test coverage.
* subversion/include/svn_string.h
(svn_stringbuf_appendbyte): declare new function
* subversion/libsvn_subr/svn_string.c
(svn_stringbuf_appendbyte): implement new function
* subversion/libsvn_client/deprecated.c
(wrapped_receiver): replace call to svn_stringbuf_appendbytes
with a less expensive call to svn_stringbuf_appendbyte
* subversion/libsvn_diff/diff_file.c
(output_unified_line): dito.
* subversion/libsvn_diff/parse-diff.c
(parse_hunk_header): dito.
* subversion/libsvn_fs_fs/lock.c
(write_digest_file): dito.
* subversion/libsvn_ra_svn/marshal.c
(read_item): dito.
* subversion/libsvn_repos/dump.c
(write_hash_to_stringbuf): dito.
* subversion/libsvn_subr/config_file.c
(parse_value, parse_option, parse_section_name): dito.
* subversion/libsvn_subr/prompt.c
(prompt): dito.
* subversion/libsvn_subr/quoprint.c
(encode_bytes, decode_bytes): dito.
* subversion/libsvn_subr/skel.c
(unparse): dito.
* subversion/libsvn_subr/stream.c
(stream_readline): dito.
* subversion/libsvn_subr/subst.c
(keyword_printf): dito.
* subversion/libsvn_wc/props.c
(svn_wc_canonicalize_svn_prop): dito
* subversion/svn/util.c
(svn_cl__get_log_message): dito
* subversion/svndumpfilter/main.c
(write_prop_to_stringbuf, output_revision, output_node): dito.
* subversion/tests/libsvn_subr/skel-test.c
(put_implicit_length_byte, put_implicit_length_all_chars,
put_list_start, put_list_end): dito.
* subversion/tests/libsvn_subr/stream-test.c
(generate_test_bytes): dito.
patch by stefanfuhrmann < at > alice-dsl.de
]]]
Index: subversion/include/svn_string.h
===================================================================
--- subversion/include/svn_string.h (revision 939976)
+++ subversion/include/svn_string.h (working copy)
@@ -253,6 +253,15 @@
void
svn_stringbuf_fillchar(svn_stringbuf_t *str, unsigned char c);
+/** Append a single character onto @a targetstr.
+ *
+ * reallocs if necessary. @a targetstr is affected, nothing else is.
+ * @since New in 1.7.
+ */
+void
+svn_stringbuf_appendbyte(svn_stringbuf_t *targetstr,
+ char byte);
+
/** Append an array of bytes onto @a targetstr.
*
* reallocs if necessary. @a targetstr is affected, nothing else is.
Index: subversion/libsvn_client/deprecated.c
===================================================================
--- subversion/libsvn_client/deprecated.c (revision 939976)
+++ subversion/libsvn_client/deprecated.c (working copy)
@@ -277,7 +277,7 @@
struct wrapped_receiver_baton_s *b = baton;
svn_stringbuf_t *expanded_line = svn_stringbuf_create(line, pool);
- svn_stringbuf_appendbytes(expanded_line, "\r", 1);
+ svn_stringbuf_appendbyte(expanded_line, '\r');
return b->orig_receiver(b->orig_baton, line_no, revision, author,
date, expanded_line->data, pool);
Index: subversion/libsvn_diff/diff_file.c
===================================================================
--- subversion/libsvn_diff/diff_file.c (revision 939976)
+++ subversion/libsvn_diff/diff_file.c (working copy)
@@ -860,7 +860,7 @@
{
if (type != svn_diff__file_output_unified_skip)
{
- svn_stringbuf_appendbytes(baton->hunk, curp, 1);
+ svn_stringbuf_appendbyte(baton->hunk, *curp);
}
/* We don't append the LF to extra_context, since it would
* just be stripped anyway. */
Index: subversion/libsvn_diff/parse-diff.c
===================================================================
--- subversion/libsvn_diff/parse-diff.c (revision 939976)
+++ subversion/libsvn_diff/parse-diff.c (working copy)
@@ -124,7 +124,7 @@
p++;
while (*p && *p != ' ')
{
- svn_stringbuf_appendbytes(range, p, 1);
+ svn_stringbuf_appendbyte(range, &p);
p++;
}
if (*p != ' ')
@@ -153,7 +153,7 @@
p++;
while (*p && *p != ' ')
{
- svn_stringbuf_appendbytes(range, p, 1);
+ svn_stringbuf_appendbyte(range, &p);
p++;
}
if (*p != ' ')
Index: subversion/libsvn_fs_fs/lock.c
===================================================================
--- subversion/libsvn_fs_fs/lock.c (revision 939976)
+++ subversion/libsvn_fs_fs/lock.c (working copy)
@@ -192,7 +192,7 @@
svn_stringbuf_appendbytes(children_list,
svn__apr_hash_index_key(hi),
svn__apr_hash_index_klen(hi));
- svn_stringbuf_appendbytes(children_list, "\n", 1);
+ svn_stringbuf_appendbyte(children_list, '\n');
}
hash_store(hash, CHILDREN_KEY, sizeof(CHILDREN_KEY)-1,
children_list->data, children_list->len, pool);
Index: subversion/libsvn_ra_svn/marshal.c
===================================================================
--- subversion/libsvn_ra_svn/marshal.c (revision 939976)
+++ subversion/libsvn_ra_svn/marshal.c (working copy)
@@ -630,7 +630,7 @@
SVN_ERR(readbuf_getchar(conn, pool, &c));
if (!apr_isalnum(c) && c != '-')
break;
- svn_stringbuf_appendbytes(str, &c, 1);
+ svn_stringbuf_appendbyte(str, c);
}
item->kind = SVN_RA_SVN_WORD;
item->u.word = str->data;
Index: subversion/libsvn_repos/dump.c
===================================================================
--- subversion/libsvn_repos/dump.c (revision 939976)
+++ subversion/libsvn_repos/dump.c (working copy)
@@ -84,7 +84,7 @@
keylen));
svn_stringbuf_appendbytes(*strbuf, (const char *) key, keylen);
- svn_stringbuf_appendbytes(*strbuf, "\n", 1);
+ svn_stringbuf_appendbyte(*strbuf, '\n');
/* Output value length, then value. */
@@ -93,7 +93,7 @@
value->len));
svn_stringbuf_appendbytes(*strbuf, value->data, value->len);
- svn_stringbuf_appendbytes(*strbuf, "\n", 1);
+ svn_stringbuf_appendbyte(*strbuf, '\n');
}
if (oldhash)
@@ -120,7 +120,7 @@
keylen));
svn_stringbuf_appendbytes(*strbuf, (const char *) key, keylen);
- svn_stringbuf_appendbytes(*strbuf, "\n", 1);
+ svn_stringbuf_appendbyte(*strbuf, '\n');
}
}
svn_stringbuf_appendbytes(*strbuf, "PROPS-END\n", 10);
Index: subversion/libsvn_subr/config_file.c
===================================================================
--- subversion/libsvn_subr/config_file.c (revision 939976)
+++ subversion/libsvn_subr/config_file.c (working copy)
@@ -162,7 +162,7 @@
/* last ch seen was ':' or '=' in parse_option. */
{
const char char_from_int = ch;
- svn_stringbuf_appendbytes(ctx->value, &char_from_int, 1);
+ svn_stringbuf_appendbyte(ctx->value, char_from_int);
SVN_ERR(parser_getc(ctx, &ch));
}
/* Leading and trailing whitespace is ignored. */
@@ -213,13 +213,12 @@
else
{
/* This is a continuation line. Read it. */
- svn_stringbuf_appendbytes(ctx->value, " ", 1);
+ svn_stringbuf_appendbyte(ctx->value, ' ');
while (ch != EOF && ch != '\n')
{
const char char_from_int = ch;
- svn_stringbuf_appendbytes(ctx->value,
- &char_from_int, 1);
+ svn_stringbuf_appendbyte(ctx->value, char_from_int);
SVN_ERR(parser_getc(ctx, &ch));
}
/* Trailing whitespace is ignored. */
@@ -246,7 +245,7 @@
while (ch != EOF && ch != ':' && ch != '=' && ch != '\n')
{
const char char_from_int = ch;
- svn_stringbuf_appendbytes(ctx->option, &char_from_int, 1);
+ svn_stringbuf_appendbyte(ctx->option, char_from_int);
SVN_ERR(parser_getc(ctx, &ch));
}
@@ -289,7 +288,7 @@
while (ch != EOF && ch != ']' && ch != '\n')
{
const char char_from_int = ch;
- svn_stringbuf_appendbytes(ctx->section, &char_from_int, 1);
+ svn_stringbuf_appendbyte(ctx->section, char_from_int);
SVN_ERR(parser_getc(ctx, &ch));
}
Index: subversion/libsvn_subr/prompt.c
===================================================================
--- subversion/libsvn_subr/prompt.c (revision 939976)
+++ subversion/libsvn_subr/prompt.c (working copy)
@@ -147,7 +147,7 @@
SVN_ERR_MALFUNCTION();
}
- svn_stringbuf_appendbytes(strbuf, &c, 1);
+ svn_stringbuf_appendbyte(strbuf, c);
}
}
else
Index: subversion/libsvn_subr/quoprint.c
===================================================================
--- subversion/libsvn_subr/quoprint.c (revision 939976)
+++ subversion/libsvn_subr/quoprint.c (working copy)
@@ -92,7 +92,7 @@
/* Encode this character. */
if (ENCODE_AS_LITERAL(*p))
{
- svn_stringbuf_appendbytes(str, p, 1);
+ svn_stringbuf_appendbyte(str, *p);
(*linelen)++;
}
else
@@ -218,7 +218,7 @@
{
/* Literal character; append it if it's valid as such. */
if (VALID_LITERAL(*inbuf))
- svn_stringbuf_appendbytes(str, inbuf, 1);
+ svn_stringbuf_appendbyte(str, *inbuf);
*inbuflen = 0;
}
else if (*inbuf == '=' && *inbuflen == 2 && inbuf[1] == '\n')
@@ -234,7 +234,7 @@
if (find1 != NULL && find2 != NULL)
{
c = ((find1 - hextab) << 4) | (find2 - hextab);
- svn_stringbuf_appendbytes(str, &c, 1);
+ svn_stringbuf_appendbyte(str, c);
}
*inbuflen = 0;
}
Index: subversion/libsvn_subr/skel.c
===================================================================
--- subversion/libsvn_subr/skel.c (revision 939976)
+++ subversion/libsvn_subr/skel.c (working copy)
@@ -535,7 +535,7 @@
}
/* Emit a closing parenthesis. */
- svn_stringbuf_appendbytes(str, ")", 1);
+ svn_stringbuf_appendbyte(str, ')');
}
return str;
Index: subversion/libsvn_subr/stream.c
===================================================================
--- subversion/libsvn_subr/stream.c (revision 939976)
+++ subversion/libsvn_subr/stream.c (working copy)
@@ -376,7 +376,7 @@
else
match = eol_str;
- svn_stringbuf_appendbytes(str, &c, 1);
+ svn_stringbuf_appendbyte(str, c);
}
svn_stringbuf_chop(str, match - eol_str);
Index: subversion/libsvn_subr/subst.c
===================================================================
--- subversion/libsvn_subr/subst.c (revision 939976)
+++ subversion/libsvn_subr/subst.c (working copy)
@@ -211,10 +211,10 @@
svn_stringbuf_appendcstr(value, url);
break;
case '%': /* '%%' => a literal % */
- svn_stringbuf_appendbytes(value, cur, 1);
+ svn_stringbuf_appendbyte(value, *cur);
break;
case '\0': /* '%' as the last character of the string. */
- svn_stringbuf_appendbytes(value, cur, 1);
+ svn_stringbuf_appendbyte(value, *cur);
/* Now go back one character, since this was just a one character
* sequence, whereas all others are two characters, and we do not
* want to skip the null terminator entirely and carry on
Index: subversion/libsvn_subr/svn_string.c
===================================================================
--- subversion/libsvn_subr/svn_string.c (revision 939976)
+++ subversion/libsvn_subr/svn_string.c (working copy)
@@ -385,6 +385,30 @@
void
+svn_stringbuf_appendbyte(svn_stringbuf_t *str, char byte)
+{
+ /* In most cases, there will be pre-allocated memory
+ * left to just write the byte at the end of the used
+ * section and terminate the string properly.
+ */
+ apr_size_t old_len = str->len;
+ if (str->blocksize > old_len + 1)
+ {
+ str->data[old_len] = byte;
+ str->data[old_len+1] = '\0';
+ str->len = old_len+1;
+ }
+ else
+ {
+ /* we need to re-allocate the string buffer
+ * -> let the more generic implementation handle that
+ */
+ svn_stringbuf_appendbytes(str, &byte, 1);
+ }
+}
+
+
+void
svn_stringbuf_appendbytes(svn_stringbuf_t *str, const char *bytes,
apr_size_t count)
{
Index: subversion/libsvn_wc/props.c
===================================================================
--- subversion/libsvn_wc/props.c (revision 939976)
+++ subversion/libsvn_wc/props.c (working copy)
@@ -2444,7 +2444,7 @@
if (propval->data[propval->len - 1] != '\n')
{
new_value = svn_stringbuf_create_from_string(propval, pool);
- svn_stringbuf_appendbytes(new_value, "\n", 1);
+ svn_stringbuf_appendbyte(new_value, '\n');
}
/* Make sure this is a valid externals property. Do not
Index: subversion/svn/util.c
===================================================================
--- subversion/svn/util.c (revision 939976)
+++ subversion/svn/util.c (working copy)
@@ -762,9 +762,9 @@
&& item->state_flags & SVN_CLIENT_COMMIT_ITEM_LOCK_TOKEN)
unlock = 'U';
- svn_stringbuf_appendbytes(tmp_message, &text_mod, 1);
- svn_stringbuf_appendbytes(tmp_message, &prop_mod, 1);
- svn_stringbuf_appendbytes(tmp_message, &unlock, 1);
+ svn_stringbuf_appendbyte(tmp_message, text_mod);
+ svn_stringbuf_appendbyte(tmp_message, prop_mod);
+ svn_stringbuf_appendbyte(tmp_message, unlock);
if (item->state_flags & SVN_CLIENT_COMMIT_ITEM_IS_COPY)
/* History included via copy/move. */
svn_stringbuf_appendcstr(tmp_message, "+ ");
Index: subversion/svndumpfilter/main.c
===================================================================
--- subversion/svndumpfilter/main.c (revision 939976)
+++ subversion/svndumpfilter/main.c (working copy)
@@ -91,20 +91,20 @@
bytes_used = apr_snprintf(buf, sizeof(buf), "%d", namelen);
svn_stringbuf_appendbytes(*strbuf, buf, bytes_used);
- svn_stringbuf_appendbytes(*strbuf, "\n", 1);
+ svn_stringbuf_appendbyte(*strbuf, '\n');
svn_stringbuf_appendbytes(*strbuf, name, namelen);
- svn_stringbuf_appendbytes(*strbuf, "\n", 1);
+ svn_stringbuf_appendbyte(*strbuf, '\n');
/* Output value length, then value. */
svn_stringbuf_appendbytes(*strbuf, "V ", 2);
bytes_used = apr_snprintf(buf, sizeof(buf), "%" APR_SIZE_T_FMT, value->len);
svn_stringbuf_appendbytes(*strbuf, buf, bytes_used);
- svn_stringbuf_appendbytes(*strbuf, "\n", 1);
+ svn_stringbuf_appendbyte(*strbuf, '\n');
svn_stringbuf_appendbytes(*strbuf, value->data, value->len);
- svn_stringbuf_appendbytes(*strbuf, "\n", 1);
+ svn_stringbuf_appendbyte(*strbuf, '\n');
}
@@ -365,19 +365,19 @@
bytes_used = apr_snprintf(buf, sizeof(buf), ": %" APR_SIZE_T_FMT,
props->len);
svn_stringbuf_appendbytes(rb->header, buf, bytes_used);
- svn_stringbuf_appendbytes(rb->header, "\n", 1);
+ svn_stringbuf_appendbyte(rb->header, '\n');
}
svn_stringbuf_appendcstr(rb->header, SVN_REPOS_DUMPFILE_CONTENT_LENGTH);
bytes_used = apr_snprintf(buf, sizeof(buf), ": %" APR_SIZE_T_FMT, props->len);
svn_stringbuf_appendbytes(rb->header, buf, bytes_used);
- svn_stringbuf_appendbytes(rb->header, "\n", 1);
+ svn_stringbuf_appendbyte(rb->header, '\n');
/* put an end to headers */
- svn_stringbuf_appendbytes(rb->header, "\n", 1);
+ svn_stringbuf_appendbyte(rb->header, '\n');
/* put an end to revision */
- svn_stringbuf_appendbytes(props, "\n", 1);
+ svn_stringbuf_appendbyte(props, '\n');
/* write out the revision */
/* Revision is written out in the following cases:
@@ -632,7 +632,7 @@
bytes_used = apr_snprintf(buf, sizeof(buf), ": %" APR_SIZE_T_FMT,
nb->props->len);
svn_stringbuf_appendbytes(nb->header, buf, bytes_used);
- svn_stringbuf_appendbytes(nb->header, "\n", 1);
+ svn_stringbuf_appendbyte(nb->header, '\n');
}
if (nb->has_text)
{
@@ -641,16 +641,16 @@
bytes_used = apr_snprintf(buf, sizeof(buf), ": %" SVN_FILESIZE_T_FMT,
nb->tcl);
svn_stringbuf_appendbytes(nb->header, buf, bytes_used);
- svn_stringbuf_appendbytes(nb->header, "\n", 1);
+ svn_stringbuf_appendbyte(nb->header, '\n');
}
svn_stringbuf_appendcstr(nb->header, SVN_REPOS_DUMPFILE_CONTENT_LENGTH);
bytes_used = apr_snprintf(buf, sizeof(buf), ": %" SVN_FILESIZE_T_FMT,
(svn_filesize_t) (nb->props->len + nb->tcl));
svn_stringbuf_appendbytes(nb->header, buf, bytes_used);
- svn_stringbuf_appendbytes(nb->header, "\n", 1);
+ svn_stringbuf_appendbyte(nb->header, '\n');
/* put an end to headers */
- svn_stringbuf_appendbytes(nb->header, "\n", 1);
+ svn_stringbuf_appendbyte(nb->header, '\n');
/* 3. output all the stuff */
Index: subversion/tests/libsvn_subr/skel-test.c
===================================================================
--- subversion/tests/libsvn_subr/skel-test.c (revision 939976)
+++ subversion/tests/libsvn_subr/skel-test.c (working copy)
@@ -184,9 +184,9 @@
&& ! skel_is_space(term)
&& ! skel_is_paren(term))
abort();
- svn_stringbuf_appendbytes(str, &byte, 1);
+ svn_stringbuf_appendbyte(str, byte);
if (term != '\0')
- svn_stringbuf_appendbytes(str, &term, 1);
+ svn_stringbuf_appendbyte(str, term);
}
@@ -239,7 +239,7 @@
svn_stringbuf_appendbytes(str, name, len);
if (term != '\0')
- svn_stringbuf_appendbytes(str, &term, 1);
+ svn_stringbuf_appendbyte(str, term);
}
@@ -461,7 +461,7 @@
svn_stringbuf_appendcstr(str, "(");
for (i = 0; i < len; i++)
- svn_stringbuf_appendbytes(str, &space, 1);
+ svn_stringbuf_appendbyte(str, space);
}
@@ -476,7 +476,7 @@
abort();
for (i = 0; i < len; i++)
- svn_stringbuf_appendbytes(str, &space, 1);
+ svn_stringbuf_appendbyte(str, space);
svn_stringbuf_appendcstr(str, ")");
}
Index: subversion/tests/libsvn_subr/stream-test.c
===================================================================
--- subversion/tests/libsvn_subr/stream-test.c (revision 939976)
+++ subversion/tests/libsvn_subr/stream-test.c (working copy)
@@ -128,7 +128,7 @@
for (total = 0, repeat = repeat_iter = 1, c = 0; total < num_bytes; total++)
{
- svn_stringbuf_appendbytes(buffer, &c, 1);
+ svn_stringbuf_appendbyte(buffer, c);
repeat_iter--;
if (repeat_iter == 0)
Received on 2010-05-01 18:27:24 CEST