Re: [PATCH v2 04/10] packfile: inline cache_or_unpack_entry
- From
- Eric Wong <e@80x24.org>
- Date
- Oct 6, 2024, 17:40 UTC
- Message-ID
- <20241006174033.M85696@dcvr>
- In-Reply-To
- <xmqqjzg3ky77.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> wrote:
Show 14 quoted lines
> Eric Wong <e@80x24.org> writes: > > > We need to check delta_base_cache anyways to fill in the > > `whence' field in `struct object_info'. Inlining (and getting > > rid of) cache_or_unpack_entry() makes it easier to only do the > > hashmap lookup once and avoid a redundant lookup later on. > > > > This code reorganization will also make an optimization to > > use the cache entry directly easier to implement in the next > > commit. > > "cache entry" -> "cached entry"; we tend to use "cache entry" > exclusively to mean an entry in the in-core index structure, > and not the cached objects held in the object layer.
OK, will do for v3 (apologies for the delay, Real Life is awful :<).
Show 13 quoted lines
> > + type = ent->type;
> > + if (oi->sizep)
> > + *oi->sizep = ent->size;
> > + if (oi->contentp) {
> > + if (!oi->content_limit ||
> > + ent->size <= oi->content_limit)
> > + *oi->contentp = xmemdupz(ent->data, ent->size);
> > + else
> > + *oi->contentp = NULL; /* caller must stream */
>
> This assignment of NULL is more explicit than the original; is it
> because the original assumed that *(oi->contentp) is initialized to
> NULL if oi->contentp asks us to give the contents?The original code unconditionally assigned cache_or_unpack_entry() result to *oi->contentp, and there's a bunch of non-cat-file callers which pass a non-NULL *oi->contentp and expect packed_object_info() to NULL it.