From: Jeff King <peff@peff.net>
Date: Thu, 02 May 2013 15:46:18 GMT
Subject: Re: [PATCH] lookup_object: prioritize recently found objects
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:

> > 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