Re: [PATCH 3/4] pack-objects: don't traverse objects unnecessarily
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:52:20
Dan McGee [off-list ref] 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...
quoted hunk
@@ -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);