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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On Thu, 8 Nov 2007, René Scharfe wrote:

> 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*

> > 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

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux