Thomas Rast <trast@xxxxxxxxxxxxxxx> 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 <trast@xxxxxxxxxxxxxxx>
> ---
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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html