On Wed, 8 Dec 2010, Junio C Hamano wrote:
> 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.
> 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;
}
> 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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html