Re: [PATCH v2 2/3] Refactor parts of in_delta_base_cache/cache_or_unpack_entry
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:56:31
Thomas Rast [off-list ref] 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 <redacted> ---
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?
quoted hunk
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;