Re: [PATCH 3/4] pack-objects: don't traverse objects unnecessarily

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 27, 2011, 22:26 UTC
Message-ID: <7vk47qxe9x.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1318915284-6361-3-git-send-email-dpmcgee@gmail.com>

Dan McGee <dpmcgee@gmail.com> writes:

> Two optimizations take place here- we can start our objects array
> iteration from a known point where we left off before we started trying
> to find our tags,

This I would understand (but I am somewhat curious how much last_untagged would advance relative to nr_objects for this half of the optimization to be worth it), but...

> and we don't need to do the deep dives required by
> add_family_to_write_order() if the object has already been marked as
> filled.

I am not sure if this produces the identical result that was benchmarked in the original series.

For example, if you have a tagged object that is not a commit (say a blob), you would have written that blob in the second phase (write tagged objects together), so the family of blobs that share same delta parent as that blob will not be written in this "Finally all the rest" in the right place in the original list, no?

I do not think this change would forget to fill an object that needs to be filled, but it would affect the resulting ordering of the list, so...

Show 13 quoted lines

> @@ -560,8 +561,13 @@ static struct object_entry **compute_write_order(void)
>  	/*
>  	 * Finally all the rest in really tight order
>  	 */
> -	for (i = 0; i < nr_objects; i++)
> -		add_family_to_write_order(wo, &wo_end, &objects[i]);
> +	for (i = last_untagged; i < nr_objects; i++) {
> +		if (!objects[i].filled)
> +			add_family_to_write_order(wo, &wo_end, &objects[i]);
> +	}
> +
> +	if(wo_end != nr_objects)
> +		die("ordered %u objects, expected %u", wo_end, nr_objects);