[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

RE: svn commit: r1344347 - /subversion/trunk/subversion/libsvn_subr/config_file.c

From: Bert Huijben <bert_at_qqmail.nl>
Date: Tue, 25 Mar 2014 12:11:24 +0100

> -----Original Message-----
> From: Ivan Zhakov [mailto:ivan_at_visualsvn.com]
> Sent: dinsdag 25 maart 2014 12:03
> To: Subversion Development; Bert Huijben
> Subject: Re: svn commit: r1344347 -
> /subversion/trunk/subversion/libsvn_subr/config_file.c
>
> On 30 May 2012 20:52, <rhuijben_at_apache.org> wrote:
> > 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/co
> nfig_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 */
> Hi Bert,
>
> Did you have specific reasons for using deprecated
> SVN_STREAM_CHUNK_SIZE (100 kb) instead of SVN__STREAM_CHUNK_SIZE
> (16kb) constant for read buffer? I'm asking because it seems this
> significantly increase memory footprint for high loaded servers due
> APR allocator behavior and fragmentation.
>
> I would like change this buffer to 16kb instead of 100kb, if you don't
> have strong objections.

Answered on IRC: +1

Using something like <= 8 KByte would not handle our default generated client side config files...
But 16 KB will easily hold them.

The fsfs and repository config files are much smaller, so if it really helps we could make this configurable per use case.

        Bert
Received on 2014-03-25 12:12:12 CET

This is an archived mail posted to the Subversion Dev mailing list.