Re: [PATCH 3/3] pretty=format: Avoid some expensive calculations when not needed

From: Pierre Habouzit <madcoder@debian.org>
Date: Nov 7, 2007, 00:11 UTC
Message-ID: <20071107001112.GD4382@artemis.corp>
In-Reply-To: <4730F5FA.3030705@lsrfire.ath.cx>

On Tue, Nov 06, 2007 at 11:17:14PM +0000, René Scharfe wrote:

> I haven't seen any comments on strbuf_expand.  Is it too far out?
> Here it is again, adjusted for current master and with the changes
> to strbuf.[ch] coming first:

  I have one.

Show 24 quoted lines

>  strbuf.c |   22 +++++
>  strbuf.h |    3 
>  pretty.c |  276 ++++++++++++++++++++++++++++++++++-----------------------------
>  3 files changed, 178 insertions(+), 123 deletions(-)
> 
> diff --git a/strbuf.c b/strbuf.c
> index f4201e1..b71da99 100644
> --- a/strbuf.c
> +++ b/strbuf.c
> @@ -129,6 +129,28 @@ void strbuf_addf(struct strbuf *sb, const char *fmt, ...)
>  	strbuf_setlen(sb, sb->len + len);
>  }
>  
> +void strbuf_expand(struct strbuf *sb, const char *fmt,
> +                   const char **placeholders, expand_fn_t fn, void *context)
> +{
> +	char c;
> +	const char **p;
> +
> +	while ((c = *fmt++)) {
> +		if (c != '%') {
> +			strbuf_addch(sb, c);
> +			continue;
> +		}

strbuf_addch is pretty inneficient as it puts NULs each time. rather do that (sketchy) :

{
    for (;;) {
        const char *percent = strchr(fmt, '%');
        if (!percent)
            break;
        strbuf_add(sb, fmt, percent - fmt);
        fmt = percent + 1;

        /* do your stuff */
    }
    strbuf_addstr(sb, fmt);
}

Of course it's a detail as formats will probably be short. But it's a good example to show to people wanting to write new strbuf functions.

This nitpicking apart, the timings are impressive.

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org