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

On 25 March 2014 15:11, Bert Huijben <bert_at_qqmail.nl> wrote:
>
>
>> -----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,

Thanks for quick reply.

Committed in r1581296 and nominated for backport to 1.8.x.

-- 
Ivan Zhakov
CTO | VisualSVN | http://www.visualsvn.com