Jonathan Nieder <jrnieder@xxxxxxxxx> writes: > Yay, thanks for this. > > When this condition triggers (count >= 10240), we have already > experienced 10 rounds of negotiation. Negotiation ought to have > finished by then. So this is a pretty conservative change to try to > salvage an already bad situation. > > The condition ensures that the exponential growth will go faster > than the previous heuristic of linear growth. > > Memory usage grows with the number of 'have's to be sent. Linear > growth didn't bound memory usage. This exponential growth makes memory > usage increase faster, but not aggressively so and the unbounded > memory usage is already something we'd want to address separately to > handle hostile servers. > > All in all, this looks likely to allow negotiation to finish in fewer > rounds, speeding up fetch, without much downside, so for what it's > worth, > > Reviewed-by: Jonathan Nieder <jrnieder@xxxxxxxxx> > > I'd expect us to need more aggressive improvements to negotiation in the > end (e.g. finding a way to order SHA-1s sent as 'have's to finish in > fewer rounds). But this is a good start. Thanks for writing it. Sorry, while I agree with the general sentiment that the windowing heuristics can be improved, from your description, I would have expected an updated curve goes like "aggressive exponential -> conservative exponential -> slow linear", but the new comparison reads the other way around, i.e. "aggressive exponential -> slow linear -> conservative exponential". I'd understand if it were more like "aggressive exponential -> conservative exponential" without linear phase when stateless_rpc is in use, though. I just do not quite understand the justification behind the order of three phases introduced by this change. >> diff --git a/fetch-pack.c b/fetch-pack.c >> index b501d5c..3fcbda2 100644 >> --- a/fetch-pack.c >> +++ b/fetch-pack.c >> @@ -251,6 +251,8 @@ static int next_flush(struct fetch_pack_args *args, int count) >> >> if (count < flush_limit) >> count <<= 1; >> + else if (args->stateless_rpc && count >= flush_limit * 10) >> + count = count * 11 / 10; >> else >> count += flush_limit; >> return count; -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html