From: Jeff Garzik <jeff@garzik.org>
Date: Sat, 09 Sep 2006 22:33:39 GMT
Subject: Re: {RFC/PATCH] micro-optimize get_sha1_hex()
Message-ID: <45034143.4050308@garzik.org>
In-Reply-To: <7vzmd8vh6q.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano wrote:
>  int get_sha1_hex(const char *hex, unsigned char *sha1)
>  {
>  	int i;
>  	for (i = 0; i < 20; i++) {
> -		unsigned int val = (hexval(hex[0]) << 4) | hexval(hex[1]);
> -		if (val & ~0xff)
> +		unsigned int v, w, val;
> +		v = *hex++;
> +		if ((v < '0') || ('f' < v) ||
> +		    ((v = hexval[v-'0']) == 255))
> +			return -1;
> +		w = *hex++;
> +		if ((w < '0') || ('f' < w) ||
> +		    ((w = hexval[w-'0']) == 255))
>  			return -1;
> -		*sha1++ = val;
> -		hex += 2;
> +		*sha1++ = (v << 4) | w;

Why not just make the table include the range of all possible 
characters?  That would eliminate some comparisons and subtractions 
against '0'.

The end result would look more like the current (unpatched) form, except 
with a function replaced by the table.

	Jeff