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

From: René Scharfe <rene.scharfe@lsrfire.ath.cx>
Date: Nov 7, 2007, 23:19 UTC
Message-ID: <47324810.2010507@lsrfire.ath.cx>
In-Reply-To: <Pine.LNX.4.64.0711062343050.4362@racer.site>

Johannes Schindelin schrieb:

Show 12 quoted lines

> Hi,
> 
> 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.

Show 16 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.

René