Re: [PATCH] gc: remove redundant check for gc_auto_threshold

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Wed, Oct 10 2018, Junio C Hamano wrote:

> Brandon Casey <drafnel@gmail.com> writes:
>
>> ...  Again, I don't feel strongly about it, but I'm not
>> sure this change actually improves the code.
>
> Yeah, in the context of the current caller, this is a safe change
> that does not break anybody and reduces the number of instructions
> executed in this codepath.  A mistaken caller may be added in the
> future that fails to check auto-threashold beforehand, but that
> won't lead to anything bad like looping for a large number of times,
> so as long as the API contract into this helper function is clear
> that callers are responsible to check beforehand, it is still not
> too bad.
>
> So, I'd throw this into "Meh - I won't regret applying it, but it is
> not the end of the world if I forget to apply it, either" pile.
>
> I _think_ a change that actually improves the code would be to
> restructure so that there is a helper that is responsible for
> guestimating the number of loose objects, and another that uses the
> helper to see if there are too many loose objects.  The latter is
> the only one tha needs to know about auto-threashold.  But we are
> not in immdiate need for such a clean-up, I guess, unless somebody
> is actively looking into revamping how auto-gc works and doing a
> preparatory clean-up.

Yeah, that's me :) I have some WIP gc cleanup, but want to sit on it a
bit before I submit it to think about the best way to do things.

So in the meantime I was sending out a few WIP bits that I expected
could be reviewed stand-alone.

So I'd prefer to have this applied. It's easy enough to understand that
shouldn't take long to prove to be correct & trickle down to "master",
and will make those subsequent patches easier to follow.