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

From: Jeff King <peff@peff.net>
Date: Sep 4, 2025, 11:22 UTC
Message-ID: <20250904112255.GB2474144@coredump.intra.peff.net>
In-Reply-To: <aeb6aa64-c2f8-4b28-a2cf-9662907b5feb@web.de>

On Wed, Sep 03, 2025 at 06:30:57PM +0200, René Scharfe wrote:

Show 12 quoted lines

> On 8/25/25 9:34 AM, Jeff King wrote:
> > 
> > [oidset instead of khash]
> > But the bonus here is that we didn't need to define a new hash
> > type, nor do any tricks with the commit->index field.
> 
> It took me a while to notice what's tricky, or rather inconsistent,
> about the khash set of commit objects with commit index as hash: We
> could just as well go all in and use an uint32_t khash set of commit
> indexes.  That would reduce the memory footprint further.  No pointers
> needed here.  Didn't measure a meaningful performance difference
> though, so that's that..

Heh, yes, that's true. I didn't even think about that. Conceptually we could replace "struct object" with just an index value, and then all code could just allocate arrays and index them. And the commit-slab code is essentially a slightly more dynamic version of that. And a hash of ints is a good way of handling a sparsely populated set of data.

It's less crazy than it sounds because we allocate objects sequentially and never free them anyway. But it would still be a pretty big change (and each data access would involve a few extra instructions to compute the address).

Anyway, I'm happy enough with the oidset solution you landed on.

-Peff