Re: 2.18.0 Regression: packing performance and effectiveness

[Junio C Hamano <gitster@pobox.com>]

Duy Nguyen <pclouds@gmail.com> writes:

> @@ -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?