git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v3] unpack-trees: avoid duplicate ODB lookups during checkout

From
Jeff King <peff@peff.net>
Date
Apr 14, 2017, 00:59 UTC
Message-ID
<20170414005924.sfu4csdxaoafipsv@sigill.intra.peff.net>
In-Reply-To
<xmqqtw5roqcc.fsf@gitster.mtv.corp.google.com>
On Thu, Apr 13, 2017 at 04:11:31PM -0700, Junio C Hamano wrote:
Show 26 quoted lines
> git@jeffhostetler.com 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).

-Peff
Previous: Junio C HamanoNext: Jeff Hostetler
Message 4 of 5 in “unpack-trees: avoid duplicate ODB lookups during checkout”
  1. unpack-trees: avoid duplicate ODB lookups during checkoutgit@jeffhostetler.com, Apr 13, 2017
  2. unpack-trees: avoid duplicate ODB lookups during checkoutgit@jeffhostetler.com, Apr 13, 2017
  3. Junio C HamanoApr 13, 2017
  4. Jeff KingApr 14, 2017
  5. Jeff HostetlerApr 14, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.