Re: [PATCH 03/16] odb: fix unnecessary call to `find_cached_object()`

[Patrick Steinhardt <ps@pks.im>]

On Wed, Apr 08, 2026 at 04:13:45PM -0500, Justin Tobler wrote:
> On 26/04/03 08:01AM, Patrick Steinhardt wrote:
> > diff --git a/odb.c b/odb.c
> > index d321242353..21cdedc31c 100644
> > --- a/odb.c
> > +++ b/odb.c
> > @@ -774,8 +774,7 @@ int odb_pretend_object(struct object_database *odb,
> >  	char *co_buf;
> >  
> >  	hash_object_file(odb->repo->hash_algo, buf, len, type, oid);
> > -	if (odb_has_object(odb, oid, 0) ||
> > -	    find_cached_object(odb, oid))
> > +	if (odb_has_object(odb, oid, 0))
> 
> Nice, odb_has_object() does indeed already check the object cache so
> that makes the explicit find_cached_object() redundant.
> 
> If a future where temporary objects could be written to the inmemory ODB
> source, would there ever be a reason for odb_has_object() to
> differentiate between inmemory and real objects?

We could in theory just append the in-memory source to the normal list
of sources, and that would ensure that all the usual operations would
know to also consider this source. But there's a couple of points that
speak against it, at least for now:

  - Callers that explicitly want to explicitly write temporary objects
    need to have a handle to the in-memory source. That handle would be
    hard to obtain if we were to only store the source in the list of
    sources.

  - It would be a change in behaviour if functions like
    `odb_for_each_object()` were to also enumerate in-memory objects.

The former one could be solved by having both the direct pointer and
keep the source in the list. The latter can be solved by having a
separate flag for `odb_for_each_object()` that tells the ODB that we
want to exclude/include in-memory objects.

But overall it feels like this would only complicate things without much
of a tangible benefit.

Patrick