Re: 2.18.0 Regression: packing performance and effectiveness
From: Junio C Hamano <hidden>
Date: 2018-07-19 19:26:34
Duy Nguyen [off-list ref] writes:
quoted hunk
@@ -330,20 +330,27 @@ static inline void oe_set_size(struct packing_data *pack, static inline unsigned long oe_delta_size(struct packing_data *pack, const struct object_entry *e) { - if (e->delta_size_valid) + if (!pack->delta_size) return e->delta_size_; - return oe_size(pack, e); + return pack->delta_size[e - pack->objects]; } +void oe_prepare_delta_size_array(struct packing_data *pack); static inline void oe_set_delta_size(struct packing_data *pack, struct object_entry *e, unsigned long size) { e->delta_size_ = size; - e->delta_size_valid = e->delta_size_ == size; - if (!e->delta_size_valid && size != oe_size(pack, e)) - BUG("this can only happen in check_object() " - "where delta size is the same as entry size"); + if (!pack->delta_size && e->delta_size_ == size) + return; + /* + * We have had at least one delta size exceeding OE_DELTA_SIZE_BITS + * limit. delta_size_ will not be used anymore. All delta sizes are now + * from the delta_size[] array. + */ + if (!pack->delta_size) + oe_prepare_delta_size_array(pack); + pack->delta_size[e - pack->objects] = size; }
Imagine that you create a delta and set its size (which happens to fit in the entry) and then create another whose size does not fit in the entry. How does oe_delta_size() know not to look at pack->delta_size[] and instead look at e->delta_size_ when it wants to know the delta for the first one? IOW, shouldn't there be a "backfill" procedure when oe_set_delta_size() notices that we now switch to pack->delta_size[] array?