Re: [PATCH 3/3] pretty=format: Avoid some expensive calculations when not needed
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- Nov 8, 2007, 00:14 UTC
- Message-ID
- <Pine.LNX.4.64.0711080012510.4362@racer.site>
- In-Reply-To
- <47324810.2010507@lsrfire.ath.cx>
Hi,
On Thu, 8 Nov 2007, René Scharfe wrote:
Show 16 quoted lines
> Johannes Schindelin schrieb: > > > On Wed, 7 Nov 2007, René Scharfe wrote: > > > >> By the way, the more intrusive surgery required when using > >> strbuf_expand() leads to even faster operation. Here my measurements > >> of most of Paul's test cases (best of three runs): > >> > >> [...] > > > > impressive timings. Although I wonder where the time comes from, as > > the other substitutions should not be _that_ expensive. > > I haven't run a profiler, but my two suspects are the malloc()s and > free()s done by interp_set_entry(), and the fact that > format_commit_message() calls interpolate() twice.
But of course! *slapsthehead*
Show 19 quoted lines
> > In any case, your approach seems much more sensible, now that we have
> > strbuf.
> >
> >> diff --git a/strbuf.h b/strbuf.h
> >> index cd7f295..95071d5 100644
> >> --- a/strbuf.h
> >> +++ b/strbuf.h
> >> @@ -102,6 +102,9 @@ static inline void strbuf_addbuf(struct strbuf *sb, struct strbuf *sb2) {
> >> strbuf_add(sb, sb2->buf, sb2->len);
> >> }
> >>
> >> +typedef void (*expand_fn_t) (struct strbuf *sb, const char *placeholder, void *context);
> >> +extern void strbuf_expand(struct strbuf *sb, const char *fmt, const char **placeholders, expand_fn_t fn, void *context);
> >
> > I wonder if it would even faster (but maybe not half as readable) if
> > expand_fd_t got the placeholder_index instead of the placeholder.
>
> I doubt it. All this would save is one pointer dereference per
> placeholder. I haven't tried and measured this, though.My thinko. I thought that you save one prefixcmp(), but strbuf_expand() offloads that to the callback function.
Thanks, Dscho