Re: [PATCH] describe: use khash in finish_depth_computation()

On Mon, Aug 25, 2025 at 11:48:09AM -0700, Junio C Hamano wrote:

Jeff King [off-list ref] writes:

quoted

So out of curiosity I tried replacing a slab that should be pretty
densely filled, using a khash based on oidhash/ptr along with some
quality-of-life wrappers.  Patch is below.

It performs...very badly. Not sure if I've screwed something up, but
it's about 7x slower to run "git rev-list --author-date-order HEAD" in
the kernel. So maybe slabs really are worth it overall.

Hmph.  It is the best case scenario for the slab code, as you'd need
author date for each and every commit object in this use case, and
the comparison function called by prio-queue would be called for the
same object many times.

But the hash function being oidhash(), I am a bit surprised.  It
shouldn't be so much more expensive to peek at the first 4 bytes and
then do the usual hashtable thing than looking at the in-object
commit->index.  Is it a sign that the range of oidhash() is a bit
too small for a real workload?

Nah, 4 byte unsigned integer should be sufficient for the number of
objects in the kernel.

I was surprised, too. I expected it be maybe 20% slower or something.
Which really makes me think I've managed to screw up the patch, but if
so, I don't see it. I tried profiling the result, expecting to see a
bunch of extra time spent in obj_timestamp_put() or obj_timestamp_get().
But I don't. They account together for only a few percent of the
run-time, according to perf.

So I dunno. I am confused by the results, but I am not sure if I am
holding it wrong.

-Peff