Eric Wong <e@xxxxxxxxx> 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.
> {
> struct pack_window *w_curs = NULL;
> - unsigned long size;
> off_t curpos = obj_offset;
> enum object_type type;
> + struct delta_base_cache_entry *ent;
>
> /*
> * We always get the representation type, but only convert it to
> * a "real" type later if the caller is interested.
> */
> - if (oi->contentp && !oi->content_limit) {
> - *oi->contentp = cache_or_unpack_entry(r, p, obj_offset, oi->sizep,
> - &type);
> + oi->whence = OI_PACKED;
> + ent = get_delta_base_cache_entry(p, obj_offset);
> + if (ent) {
> + oi->whence = OI_DBCACHED;
OK. This is very straight-forward. It is packed but if we grabbed
it from the delta-base-cache, that is the only case we know it is
dbcached.
> + 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?
> + } else if (oi->contentp && !oi->content_limit) {
> + *oi->contentp = unpack_entry(r, p, obj_offset, &type,
> + oi->sizep);
> if (!*oi->contentp)
> type = OBJ_BAD;
Nice. The code structure is still easy to follow, even though the
if/else cascade here are organized differently with more cases (used
to be "are we peeking the contents, or not?"---now it is "do this if
we can grab from the delta base cache, do one of these other things
if we have go to the packfile").
