Re: [PATCH] ls-remote: create '--sort' option

From: Jeff King <peff@peff.net>
Date: Apr 2, 2018, 21:19 UTC
Message-ID: <20180402211920.GA32099@sigill.intra.peff.net>
In-Reply-To: <20180402200736.80161-1-haraldnordgren@gmail.com>

On Mon, Apr 02, 2018 at 10:07:36PM +0200, Harald Nordgren wrote:

Show 6 quoted lines

> @@ -108,9 +115,25 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
>  			continue;
>  		if (!tail_match(pattern, ref->name))
>  			continue;
> +
> +		struct ref_array_item *item;

We don't allow mixed declarations and code; this line needs to go at the top of the block.

Show 7 quoted lines

> +		FLEX_ALLOC_MEM(item, refname, ref->name, strlen(ref->name));
> +		item->symref = ref->symref;
> +		item->objectname = ref->old_oid;
> +
> +		REALLOC_ARRAY(array.items, array.nr + 1);
> +		array.items[array.nr++] = item;
> +	}

This will grow by one each time, which ends up doing quadratic work in the number of items (although it can often be less if the realloc is able to just extend the block). It should probably use ALLOC_GROW() to move it more aggressively.

Interestingly, the ref-filter code itself seems to have the same problem, but I couldn't measure any speedup. So either my analysis is totally wrong, or we end up reusing the block most of the time in practice.

> +
> +	if (sorting) {
> +		ref_array_sort(sorting, &array);
> +	}

Minor style nit: we usually omit the braces for a one-liner block.

The ref-filter code lets you sort on any arbitrary field. So you could do:

  git ls-remote --sort=committerdate

What should that do?

With your patch, I think we'll just get something like:

  fatal: missing object 468165c1d8a442994a825f3684528361727cd8c0 for HEAD

which is perhaps the best we can do (it might even work if you've recently fetched from the other side).

-Peff