Re: [PATCH v2 2/5] fetch: Add the "--fetch-jobs" option

From: Eric Wong <e@80x24.org>
To: Palmer Dabbelt <palmer@sifive.com>
Cc: git@vger.kernel.org, peff@peff.net, chriscool@tuxfamily.org,
	gitster@pobox.com, jonathantanmy@google.com, tboegi@web.de,
	bwilliams.eng@gmail.com, jeffhost@microsoft.com
Subject: Re: [PATCH v2 2/5] fetch: Add the "--fetch-jobs" option
Date: Tue, 13 Aug 2019 14:44:41 +0000	[thread overview]
Message-ID: <20190813144441.qw5i3zbrvebz5o7z@dcvr> (raw)
In-Reply-To: <20190812213448.2649-3-palmer@sifive.com>

Palmer Dabbelt <palmer@sifive.com> wrote:
> diff --git a/builtin/fetch.c b/builtin/fetch.c
> index 8aa6a0caf1ab..fa12ad44e7d9 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c

<snip>

> +static int next_remote_to_fetch(struct child_process *cp,
> +				struct strbuf *out,
> +				void *state_uncast,
> +				void **task_state_out)
> +{
> +	int i;
> +	struct fetch_remote *state = state_uncast;
> +	struct fetch_remote_task *task_state = NULL;
> +	const char *remote_name;
> +
> +	if (state->next_remote_index >= state->all_remotes->nr)
> +		return 0;
> +
> +	remote_name = state->all_remotes->items[state->next_remote_index].string;
> +	state->next_remote_index++;
> +
> +	/*
> +	 * Finds somewhere to store the state for a task.  This is guarnteed to
> +	 * succeed because there are always enough tasks allocated to cover the
> +	 * number that have been requested to run in parallel.  Rather than
> +	 * bothering with some sort of free list, this just brute force
> +	 * searches for a free task.  The assumption is that there aren't that
> +	 * many tasks to look through.
> +	 */
> +	for (i = 0; i < state->task_count; ++i) {
> +		if (!state->all_tasks[i].in_use) {
> +			task_state = state->all_tasks + i;
> +			break;
> +		}
> +	}

Fwiw, I added list.h, the linked-list derived from the Linux
kernel to simplify usage of free lists, queues, etc...

I think it could improve readability, too; but I'm not really
a C programmer and prefer high-level scripting languages.