Nguyễn Thái Ngọc Duy <[email protected]> writes:
> We only cache deltas when it's smaller than a certain limit. This limit
> defaults to 1000 but save its compressed length in a 64-bit field.
> Shrink that field down to 16 bits, so you can only cache 65kb deltas.
> Larger deltas must be recomputed at when the pack is written down.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <[email protected]>
> ---
> if (entry->delta_data && !pack_to_stdout) {
> - entry->z_delta_size = do_compress(&entry->delta_data,
> - entry->delta_size);
> - cache_lock();
> - delta_cache_size -= entry->delta_size;
> - delta_cache_size += entry->z_delta_size;
> - cache_unlock();
> + unsigned long size;
> +
> + size = do_compress(&entry->delta_data,
> entry->delta_size);
> + entry->z_delta_size = size;
> + if (entry->z_delta_size == size) {
It is confusing to readers to write
A = B;
if (A == B) {
/* OK, A was big enough */
} else {
/* No, B is too big to fit on A */
}
I actually was about to complain that you attempted an unrelated
micro-optimization to skip cache_lock/unlock when delta_size and
z_delta_size are the same, and made a typo. Something like:
size = do_compress(...);
if (size < (1 << OE_Z_DELTA_BITS)) {
entry->z_delta_size = size;
cache_lock();
...
cache_unlock();
} else {
FREE_AND_NULL(entry->delta_data);
entry->z_delta_size = 0;
}
would have saved me a few dozens of seconds of head-scratching.
> + cache_lock();
> + delta_cache_size -= entry->delta_size;
> + delta_cache_size += entry->z_delta_size;
> + cache_unlock();
> + } else {
> + FREE_AND_NULL(entry->delta_data);
> + entry->z_delta_size = 0;
> + }
> }
