Author: rhuijben
Date: Wed May 30 16:52:36 2012
New Revision: 1344347
URL: http://svn.apache.org/viewvc?rev=1344347&view=rev
Log:
Remove about 15% of the cpu overhead of svnserve and httpd while running the
test suite by introducing a very simple parser buffer in the config file parser
code.
The nice clean stream translation code had an 7 times overhead on this very
performance critical code path, while clients of svnserve and httpd were
waiting. (Before this patch the this code path used 22% of the cpu time,
now just 3%)
* subversion/libsvn_subr/config_file.c
(parse_context_t): Add buffer and position variables. Use EOF as no ungetc
var.
(parser_getc): Skip '\r' characters as we only need the '\n' character for our
config files. Use EOF as no ungetc var.
(parse_option,
parse_section_name): Rename pool argument to scratch_pool.
(svn_config__parse_file): Create scratch pool and store the file and parse
ctx in it. Update initialization and destroy scratch_pool after successfull
parsing.
Modified:
subversion/trunk/subversion/libsvn_subr/config_file.c
Modified: subversion/trunk/subversion/libsvn_subr/config_file.c
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/config_file.c?rev=1344347&r1=1344346&r2=1344347&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/config_file.c (original)
+++ subversion/trunk/subversion/libsvn_subr/config_file.c Wed May 30 16:52:36
2012
@@ -60,15 +60,19 @@ typedef struct parse_context_t
/* The current line in the file */
int line;
- /* Cached ungotten character - streams don't support ungetc()
- [emulate it] */
+ /* Emulate an ungetc */
int ungotten_char;
- svn_boolean_t have_ungotten_char;
- /* Temporary strings, allocated from the temp pool */
+ /* Temporary strings */
svn_stringbuf_t *section;
svn_stringbuf_t *option;
svn_stringbuf_t *value;
+
+ /* Parser buffer for getc() to avoid call overhead into several libraries
+ for every character */
+ char parser_buffer[SVN_STREAM_CHUNK_SIZE]; /* Larger than most config files
*/
+ size_t buffer_pos; /* Current position within parser_buffer */
+ size_t buffer_size; /* parser_buffer contains this many bytes */
} parse_context_t;
@@ -82,23 +86,36 @@ typedef struct parse_context_t
static APR_INLINE svn_error_t *
parser_getc(parse_context_t *ctx, int *c)
{
- if (ctx->have_ungotten_char)
- {
- *c = ctx->ungotten_char;
- ctx->have_ungotten_char = FALSE;
- }
- else
+ do
{
- char char_buf;
- apr_size_t readlen = 1;
+ if (ctx->ungotten_char != EOF)
+ {
+ *c = ctx->ungotten_char;
+ ctx->ungotten_char = EOF;
+ }
+ else if (ctx->buffer_pos < ctx->buffer_size)
+ {
+ *c = ctx->parser_buffer[ctx->buffer_pos];
+ ctx->buffer_pos++;
+ }
+ else
+ {
+ ctx->buffer_pos = 0;
+ ctx->buffer_size = sizeof(ctx->parser_buffer);
- SVN_ERR(svn_stream_read(ctx->stream, &char_buf, &readlen));
+ SVN_ERR(svn_stream_read(ctx->stream, ctx->parser_buffer,
+ &(ctx->buffer_size)));
- if (readlen == 1)
- *c = char_buf;
- else
- *c = EOF;
+ if (ctx->buffer_pos < ctx->buffer_size)
+ {
+ *c = ctx->parser_buffer[ctx->buffer_pos];
+ ctx->buffer_pos++;
+ }
+ else
+ *c = EOF;
+ }
}
+ while (*c == '\r');
return SVN_NO_ERROR;
}
@@ -111,7 +128,6 @@ static APR_INLINE svn_error_t *
parser_ungetc(parse_context_t *ctx, int c)
{
ctx->ungotten_char = c;
- ctx->have_ungotten_char = TRUE;
return SVN_NO_ERROR;
}
@@ -241,7 +257,7 @@ parse_value(int *pch, parse_context_t *c
/* Parse a single option */
static svn_error_t *
-parse_option(int *pch, parse_context_t *ctx, apr_pool_t *pool)
+parse_option(int *pch, parse_context_t *ctx, apr_pool_t *scratch_pool)
{
svn_error_t *err = SVN_NO_ERROR;
int ch;
@@ -260,7 +276,7 @@ parse_option(int *pch, parse_context_t *
ch = EOF;
err = svn_error_createf(SVN_ERR_MALFORMED_FILE, NULL,
"%s:%d: Option must end with ':' or '='",
- svn_dirent_local_style(ctx->file, pool),
+ svn_dirent_local_style(ctx->file, scratch_pool),
ctx->line);
}
else
@@ -284,7 +300,8 @@ parse_option(int *pch, parse_context_t *
* starts a section name.
*/
static svn_error_t *
-parse_section_name(int *pch, parse_context_t *ctx, apr_pool_t *pool)
+parse_section_name(int *pch, parse_context_t *ctx,
+ apr_pool_t *scratch_pool)
{
svn_error_t *err = SVN_NO_ERROR;
int ch;
@@ -303,7 +320,7 @@ parse_section_name(int *pch, parse_conte
ch = EOF;
err = svn_error_createf(SVN_ERR_MALFORMED_FILE, NULL,
"%s:%d: Section header must end with ']'",
- svn_dirent_local_style(ctx->file, pool),
+ svn_dirent_local_style(ctx->file, scratch_pool),
ctx->line);
}
else
@@ -363,91 +380,101 @@ svn_config__sys_config_path(const char *
svn_error_t *
svn_config__parse_file(svn_config_t *cfg, const char *file,
- svn_boolean_t must_exist, apr_pool_t *pool)
+ svn_boolean_t must_exist, apr_pool_t *result_pool)
{
svn_error_t *err = SVN_NO_ERROR;
- parse_context_t ctx;
+ parse_context_t *ctx;
int ch, count;
svn_stream_t *stream;
+ apr_pool_t *scratch_pool = svn_pool_create(result_pool);
- err = svn_stream_open_readonly(&stream, file, pool, pool);
+ err = svn_stream_open_readonly(&stream, file, scratch_pool, scratch_pool);
if (! must_exist && err && APR_STATUS_IS_ENOENT(err->apr_err))
{
svn_error_clear(err);
+ svn_pool_destroy(scratch_pool);
return SVN_NO_ERROR;
}
else
SVN_ERR(err);
- ctx.cfg = cfg;
- ctx.file = file;
- ctx.stream = svn_subst_stream_translated(stream, "\n", TRUE, NULL, FALSE,
- pool);
- ctx.line = 1;
- ctx.have_ungotten_char = FALSE;
- ctx.section = svn_stringbuf_create_empty(pool);
- ctx.option = svn_stringbuf_create_empty(pool);
- ctx.value = svn_stringbuf_create_empty(pool);
+ ctx = apr_palloc(scratch_pool, sizeof(*ctx));
+
+ ctx->cfg = cfg;
+ ctx->file = file;
+ ctx->stream = stream;
+ ctx->line = 1;
+ ctx->ungotten_char = EOF;
+ ctx->section = svn_stringbuf_create_empty(scratch_pool);
+ ctx->option = svn_stringbuf_create_empty(scratch_pool);
+ ctx->value = svn_stringbuf_create_empty(scratch_pool);
+ ctx->buffer_pos = 0;
+ ctx->buffer_size = 0;
do
{
- SVN_ERR(skip_whitespace(&ctx, &ch, &count));
+ SVN_ERR(skip_whitespace(ctx, &ch, &count));
switch (ch)
{
case '[': /* Start of section header */
if (count == 0)
- SVN_ERR(parse_section_name(&ch, &ctx, pool));
+ SVN_ERR(parse_section_name(&ch, ctx, scratch_pool));
else
return svn_error_createf(SVN_ERR_MALFORMED_FILE, NULL,
"%s:%d: Section header"
" must start in the first column",
- svn_dirent_local_style(file, pool),
- ctx.line);
+ svn_dirent_local_style(file,
+ scratch_pool),
+ ctx->line);
break;
case '#': /* Comment */
if (count == 0)
{
- SVN_ERR(skip_to_eoln(&ctx, &ch));
- ++ctx.line;
+ SVN_ERR(skip_to_eoln(ctx, &ch));
+ ++(ctx->line);
}
else
return svn_error_createf(SVN_ERR_MALFORMED_FILE, NULL,
"%s:%d: Comment"
" must start in the first column",
- svn_dirent_local_style(file, pool),
- ctx.line);
+ svn_dirent_local_style(file,
+ scratch_pool),
+ ctx->line);
break;
case '\n': /* Empty line */
- ++ctx.line;
+ ++(ctx->line);
break;
case EOF: /* End of file or read error */
break;
default:
- if (svn_stringbuf_isempty(ctx.section))
+ if (svn_stringbuf_isempty(ctx->section))
return svn_error_createf(SVN_ERR_MALFORMED_FILE, NULL,
"%s:%d: Section header expected",
- svn_dirent_local_style(file, pool),
- ctx.line);
+ svn_dirent_local_style(file,
+ scratch_pool),
+ ctx->line);
else if (count != 0)
return svn_error_createf(SVN_ERR_MALFORMED_FILE, NULL,
"%s:%d: Option expected",
- svn_dirent_local_style(file, pool),
- ctx.line);
+ svn_dirent_local_style(file,
+ scratch_pool),
+ ctx->line);
else
- SVN_ERR(parse_option(&ch, &ctx, pool));
+ SVN_ERR(parse_option(&ch, ctx, scratch_pool));
break;
}
}
while (ch != EOF);
/* Close the streams (and other cleanup): */
- return svn_stream_close(ctx.stream);
+ svn_pool_destroy(scratch_pool);
+ return SVN_NO_ERROR;
}
