On Wed, 8 Dec 2010, Junio C Hamano wrote:
quoted text
> You seem to have gone in a slightly different direction with this reroll.
> I am not sure if use of hash_table in this code would actually improve
> anything (aside from the general readability and "reusing code from a good
> infrastructure is a good thing" principle), though, no matter how many
> tags you have in your repository.  In the code from the earlier round,
> lookup_commit_reference_gently call in the fallback codepath to populate
> commit->util used the *obj_hash[] to quickly look up the commits with the
> same object name already.

Yes; the problem now is that, in order to avoid calling replace_name() 
excessively, I need to resolve all multiply-tagged commits in one pass, 
before I have those commits in the obj_hash.  This could have been done 
with the linked list by deleting any commit_name from the linked list the 
first time replace_name() decides it should be superseded by a different 
commit_name, but the hash table approach seems less fragile to me.  The 
hash table also has the advantage of avoiding the O(#tags * #arguments) 
complexity when describe is given many arguments.

Let me know if you’d like me to try it the other way.

quoted text
> I don't know how the above would work in the face of hash collisions.

Oh right.  I’ll fix that.

static int set_util(void *chain)
{
	struct commit_name *n;
	for (n = chain; n; n = n->next) {
		struct commit *c = lookup_commit_reference_gently(n->peeled, 1);
		if (c)
			c->util = n;
	}
	return 0;
}

quoted text
> Do we know that all the archs we will be compiled on will be happy with
> this potentially unaligned access?  hash_filespec() in diffcore-rename.c
> is written in a way to avoid such an issue, and I would feel safer to 
> see this do the same.

I’ll fix that too.

static inline unsigned int hash_sha1(const unsigned char *sha1)
{
	unsigned int hash;
	memcpy(&hash, sha1, sizeof(hash));
	return hash;
}

Anders
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html