Thomas Rast <tr...@student.ethz.ch> writes:

> The delta base cache lookup and test were shared.  Refactor them;
> we'll need both parts again.  Also, we'll use the clearing routine
> later.
>
> Signed-off-by: Thomas Rast <tr...@student.ethz.ch>
> ---

Looks like a very straight-forward rewrite.

The only little concern I may have is this cmp_* function tells us
"I found it!" by returning true, which is counter-intuitive to the
readers of the caller (not the callee).

I think it makes sense to compare delta-base-cache entries only for
equality, so eq-delta-base-cache-entry might be a better name for
it, perhaps?

>  sha1_file.c | 45 ++++++++++++++++++++++++++++++++-------------
>  1 file changed, 32 insertions(+), 13 deletions(-)
>
> diff --git a/sha1_file.c b/sha1_file.c
> index 71877a7..bd054d1 100644
> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -1756,32 +1756,51 @@ static unsigned long pack_entry_hash(struct 
> packed_git *p, off_t base_offset)
>       return hash % MAX_DELTA_CACHE;
>  }
>  
> -static int in_delta_base_cache(struct packed_git *p, off_t base_offset)
> +static struct delta_base_cache_entry *
> +get_delta_base_cache_entry(struct packed_git *p, off_t base_offset)
>  {
>       unsigned long hash = pack_entry_hash(p, base_offset);
> -     struct delta_base_cache_entry *ent = delta_base_cache + hash;
> +     return delta_base_cache + hash;
> +}
> +
> +static int cmp_delta_base_cache_entry(struct delta_base_cache_entry *ent,
> +                                   struct packed_git *p, off_t base_offset)
> +{
>       return (ent->data && ent->p == p && ent->base_offset == base_offset);
>  }
>  
> +static int in_delta_base_cache(struct packed_git *p, off_t base_offset)
> +{
> +     struct delta_base_cache_entry *ent;
> +     ent = get_delta_base_cache_entry(p, base_offset);
> +     return cmp_delta_base_cache_entry(ent, p, base_offset);
> +}
> +
> +static void clear_delta_base_cache_entry(struct delta_base_cache_entry *ent)
> +{
> +     ent->data = NULL;
> +     ent->lru.next->prev = ent->lru.prev;
> +     ent->lru.prev->next = ent->lru.next;
> +     delta_base_cached -= ent->size;
> +}
> +
>  static void *cache_or_unpack_entry(struct packed_git *p, off_t base_offset,
>       unsigned long *base_size, enum object_type *type, int keep_cache)
>  {
> +     struct delta_base_cache_entry *ent;
>       void *ret;
> -     unsigned long hash = pack_entry_hash(p, base_offset);
> -     struct delta_base_cache_entry *ent = delta_base_cache + hash;
>  
> -     ret = ent->data;
> -     if (!ret || ent->p != p || ent->base_offset != base_offset)
> +     ent = get_delta_base_cache_entry(p, base_offset);
> +
> +     if (!cmp_delta_base_cache_entry(ent, p, base_offset))
>               return unpack_entry(p, base_offset, type, base_size);
>  
> -     if (!keep_cache) {
> -             ent->data = NULL;
> -             ent->lru.next->prev = ent->lru.prev;
> -             ent->lru.prev->next = ent->lru.next;
> -             delta_base_cached -= ent->size;
> -     } else {
> +     ret = ent->data;
> +
> +     if (!keep_cache)
> +             clear_delta_base_cache_entry(ent);
> +     else
>               ret = xmemdupz(ent->data, ent->size);
> -     }
>       *type = ent->type;
>       *base_size = ent->size;
>       return ret;
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to