From: Thell Fowler <git@tbfowler.name>
Date: Mon, 24 Aug 2009 03:26:56 GMT
Subject: Re: [PATCH-v2/RFC 3/6] xutils: fix ignore-all-space on incomplete line
Message-ID: <alpine.DEB.2.00.0908232044060.29625@GWPortableVCS>
In-Reply-To: <7vzl9qtev0.fsf@alter.siamese.dyndns.org>

Junio C Hamano (gitster@pobox.com) wrote on Aug 23, 2009:

> Thell Fowler <git@tbfowler.name> writes:
> 
> >> Or we can just move the final else clause up and start the function like
> >> this:
> >> 
> >> 	int i1, i2;
> >> 
> >> 	if (!(flags & XDF_WHITESPACE_FLAGS))
> >>  		return s1 == s2 && !memcmp(l1, l2, s1);
> >> 
> >> 	i1 = i2 = 0;
> >>  	if (flags & XDF_IGNORE_WHITESPACE) {
> >> 		...
> >> 
> >> that would get rid of two unnecessary clearing of variables (i1 and i2,
> >> even though I suspect that the compiler _could_ optimize them out without
> >> such an change), and three flags-bit check in the most common case of not
> >> ignoring any whitespaces.
> >> 
> >
> > HA!  That's a nifty way to do that with the variables.
> 
> My tentative draft to replace the "how about this" patch further reworks
> the loop structure and currently looks like this.
> 
> It adds net 15 lines but among that 12 lines are comments, which is not so
> bad.
> 

It passed every test I threw at it, although it seemed to be a tad bit 
slower than the previous revision on my sample data so I ran the following 
command several times for both the previous and current version:

time for i in {1..10}; do ./t4015-diff-whitespace.sh>/dev/null && 
./t4015-diff-trailing-whitespace.sh >/dev/null; done


And these results are fairly average on what I saw:

Previous version:
real	2m32.669s
user	0m44.051s
sys	1m34.702s


Current version:
real	2m56.818s
user	0m47.671s
sys	1m46.723s

--
Thell