Re: [PATCH 1/4] reftable/stack: don't perform auto-compaction with less than two tables

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

Junio C Hamano <gitster@pobox.com> writes:

> Patrick Steinhardt <ps@pks.im> writes:
>
>> In order to compact tables we need at least two tables. Bail out early
>> from `reftable_stack_auto_compact()` in case we have less than two
>> tables.
>
> While that is very true, bailing out on "< 2" would change the
> behaviour.  Where there were only one table, earlier we still went
> ahead and exercised compaction code path, but now we no longer do.
> The end result of having just a single table might logically be the
> same with this change, but if we were relying on some side effects
> of exercising the compaction code path, do we know that the rest of
> the code is OK?
>
> That's the kind of questions I would ask, if this were somebody who
> hasn't been deeply involved in the reftable code and came this deep
> in the pre-release period.  But since we all know you have been the
> main driver for this effort, we'd take your word for it ;-)
>
> Thanks, will queue.

And with code inspection, it can trivially seen that this change is
perfectly fine.

In the original, when "== 1", stack_table_sizes_for_compaction(st)
yields an array with a single element, suggest_compaction_segment()
gives back segment with .start and .end both set to 0, for which
segment_size() returns 0 hence we do not call stack_compact_range().
The original code happens to do the same when "== 0" (and 0-sized
allocation does not give back NULL).

So bypassing all of these when "== 1" is a no-op change, an
optimization to avoid allocating a single-element array and then
immediately freeing it.  Doing so when "== 0" is a strict
improvement on a platform with malloc that returns NULL for 0-sized
allocation.  So bypassing when "< 2" is totally safe and justifyable
change.

Thanks.