Re: [PATCH v3 04/14] commit-graph: implement write_commit_graph()

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 15, 2018, 18:19 UTC
Message-ID: <xmqqinay9kv4.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <1518122258-157281-5-git-send-email-dstolee@microsoft.com>

Derrick Stolee <stolee@gmail.com> writes:

Show 5 quoted lines

> +struct packed_oid_list {
> +	struct object_id **list;
> +	int nr;
> +	int alloc;
> +};

What is the typical access pattern for this data structure? If it is pretty much "allocate and grow as we find more", then a dynamic array of struct (rather than a dynamic array of pointers to struct) would be a lot more appropriate. IOW

	struct packed_oid_list {
		struct object_id *list;
		int nr, alloc;
	};

The version in the posted patch has to pay malloc overhead plus an extra pointer for each object id in the list; unless you often replace elements in the list randomly and/or you borrow object ID field in other existing data structure whose lifetime is longer than this list by pointing at it, I do not see how the extra indirection is worth it.