Re: [PATCH] lookup_object: prioritize recently found objects

From: Jeff King <peff@peff.net>
Date: May 2, 2013, 15:46 UTC
Message-ID: <20130502154618.GA18462@sigill.intra.peff.net>
In-Reply-To: <5182101D.4050807@viscovery.net>

On Thu, May 02, 2013 at 09:05:01AM +0200, Johannes Sixt wrote:

Show 7 quoted lines

> > I figured the lengthy description in the commit message would be
> > sufficient,
> 
> It's absolutely sufficient *if* one reads the commit message. In this
> case, though it goes more like "this function should be trivial, and it is
> -- up to this if statement; what the heck is it good for?" and the reader
> is forced to dig the history.

What I really didn't want to do was to rewrite the whole rationale again. I think something like:

  /*
   * Move the found object to the front of the collision chain;
   * this is a pure optimization that can make future lookups cheaper,
   * assuming there is some time-locality to which objects are looked
   * up.
   */

would be enough, and interested parties can go to the commit history (I tend to go there pretty rapidly if there is a chunk of code that sticks out, but I guess not everybody does).

> BTW, do you notice that the function is now modifying an object (the hash
> table) even though this is rather unexpected from a "lookup" function?

I think this is fine. The function is conceptually constant from the outside; callers don't even know about the hash table. They just know that there is some mapping. It's similar to the way that lookup_commit will lazily allocate the "struct commit". The callers do not care whether it exists already or not; they care that at the end of the function, they have a pointer to the commit. Everything else is an implementation detail.

-Peff