On 4/13/2017 8:59 PM, Jeff King wrote:
On Thu, Apr 13, 2017 at 04:11:31PM -0700, Junio C Hamano wrote:
git@xxxxxxxxxxxxxxxxx writes:
+ /*
+ * Fetch the tree from the ODB for each peer directory in the
+ * n commits.
+ *
+ * For 2- and 3-way traversals, we try to avoid hitting the
+ * ODB twice for the same OID. This should yield a nice speed
+ * up in checkouts and merges when the commits are similar.
+ *
+ * We don't bother doing the full O(n^2) search for larger n,
+ * because wider traversals don't happen that often and we
+ * avoid the search setup.
+ *
+ * When 2 peer OIDs are the same, we just copy the tree
+ * descriptor data. This implicitly borrows the buffer
+ * data from the earlier cell.
This "borrowing" made me worried, but it turns out that this is
perfectly OK.
fill_tree_descriptor() uses read_sha1_file() to give a tree_desc its
own copy of the tree object data, so the code that calls into the
tree traversal API is responsible for freeing the buffer returned
from fill_tree_descriptor(). The updated code avoids double freeing
by setting buf[i] to NULL for borrowing [i].
Yeah, I think that logic is fine. We could also keep a separate counter
for the size of buf, like:
buf[nr_buf++] = fill_tree_descriptor(t+i, sha1);
and then just count from zero to nr_buf in the freeing loop. I don't
think it matters much either way (the free(NULL) calls are presumably
cheap).
Agreed. I don't think this matters one way or the other, but
I'll go ahead and do this while it is fresh.
Thanks,
Jeff