Re: [PATCH] Make strbuf_cmp inline, constify its arguments and optimize it a bit

From: David Kastrup <dak@gnu.org>
Date: Oct 7, 2007, 16:27 UTC
Message-ID: <851wc6lwkc.fsf@lola.goethe.zz>
In-Reply-To: <20071007161012.GB3270@steel.home>

Alex Riesen <raa.lkml@gmail.com> writes:

Show 34 quoted lines

> David Kastrup, Sun, Oct 07, 2007 16:24:57 +0200:
>> Alex Riesen <raa.lkml@gmail.com> writes:
>> 
>> > It is definitely less code (also object code). It is not always
>> > measurably faster (but mostly is).
>> 
>> > -int strbuf_cmp(struct strbuf *a, struct strbuf *b)
>> > -{
>> > -	int cmp;
>> > -	if (a->len < b->len) {
>> > -		cmp = memcmp(a->buf, b->buf, a->len);
>> > -		return cmp ? cmp : -1;
>> > -	} else {
>> > -		cmp = memcmp(a->buf, b->buf, b->len);
>> > -		return cmp ? cmp : a->len != b->len;
>> > -	}
>> > -}
>> > -
>> 
>> > +static inline int strbuf_cmp(const struct strbuf *a, const struct strbuf *b)
>> > +{
>> > +	int len = a->len < b->len ? a->len: b->len;
>> > +	int cmp = memcmp(a->buf, b->buf, len);
>> > +	if (cmp)
>> > +		return cmp;
>> > +	return a->len < b->len ? -1: a->len != b->len;
>> > +}
>> 
>> My guess is that you are conflating two issues about speed here: the
>> inlining will like speed the stuff up.  But having to evaluate the
>> (a->len < b->len) comparison twice will likely slow it down.
>
> Can't the result of the expression be reused in compiled?
> Isn't it a common expression?

No, since the call to memcmp might change a->len or b->len. A standard-compliant C compiler can't make assumptions about what memcmp might or might not touch unless both a and b can be shown to refer to variables with an address never passed out of the scope of the compilation unit.

>> So if you do any profiling, you should do it on both separate
>> angles of this patch.
>
> I compared the inlined versions of both.

Interesting.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum