(+cc: completion people, who might test; Thomas, who knows this
code well)
Anders Kaseorg wrote:
> This makes âgit describe --exact-match HEADâ about 15 times faster on
> a cold cache (2.3s instead of 35s) in a linux-2.6 repository with many
> packed tags. Thatâs a huge win for the interactivity of the __git_ps1
> shell prompt helper.
Nice.
> --- a/builtin/describe.c
> +++ b/builtin/describe.c
> @@ -22,7 +22,7 @@ static int tags; /* Allow lightweight tags */
> static int longformat;
> static int abbrev = DEFAULT_ABBREV;
> static int max_candidates = 10;
> -static int found_names;
> +struct commit_name *names = NULL;
Nits:
- static?
- the convention in git is to leave off the zero-initializers
for bss-allocated vars (static and globals).
> @@ -34,6 +34,8 @@ static const char *diff_index_args[] = {
>
>
> struct commit_name {
> + struct commit_name *next;
> + unsigned char peeled[20];
> struct tag *tag;
> unsigned prio:2; /* annotated tag = 2, tag = 1, head = 0 */
> unsigned name_checked:1;
Hm, so the tags read so far will be stored in the "names" list.
> @@ -240,7 +231,12 @@ static void describe(const char *arg, int last_one)
> if (!cmit)
> die("%s is not a valid '%s' object", arg, commit_type);
>
> - n = cmit->util;
> + n = NULL;
> + for (e = names; e; e = e->next) {
> + if (hashcmp(e->peeled, cmit->object.sha1) == 0 &&
The usual convention is to use !hashcmp(...) to match the !strcmp(...)
idiom.
> + replace_name(n, e->prio, e->sha1, &e->tag))
> + n = e;
> + }
Instead of looking up the commit to be matched exactly in the commits
hash table, this makes a linear search.
No change to the assymptotic running time, but would this make things
much slower in the case of many tags? How many before it's a problem
(if ever)? (If it's a problem in ordinary cases, I think the
optimization could be limited to --exact-match pretty easily.)
> @@ -259,6 +255,12 @@ static void describe(const char *arg, int last_one)
> if (debug)
> fprintf(stderr, "searching to describe %s\n", arg);
>
> + for (e = names; e; e = e->next) {
> + struct commit *c = lookup_commit_reference_gently(e->peeled, 1);
> + if (c && replace_name(c->util, e->prio, e->sha1, &e->tag))
> + c->util = e;
> + }
Filling the commits table in preparation for object traversal. Now
the world's back to normal.
> @@ -418,8 +420,8 @@ int cmd_describe(int argc, const char **argv, const char *prefix)
> return cmd_name_rev(i + argc, args, prefix);
> }
>
> - for_each_ref(get_name, NULL);
> + for_each_rawref(get_name, NULL);
Orthogonal change snuck in?
> - if (!found_names && !always)
> + if (!names && !always)
Everything else looks good and very readable.
--
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