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

[Brandon Casey <drafnel@gmail.com>]

On Wed, Oct 10, 2018 at 4:38 PM Junio C Hamano <gitster@pobox.com> 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.

Agreed on all points, and as usual, said better than I could :-)

-Brandon