On Wed, Oct 10, 2018 at 4:38 PM Junio C Hamano <gitster@xxxxxxxxx> wrote: > > Brandon Casey <drafnel@xxxxxxxxx> 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
