Re: [PATCH v2 3/8] fetch-pack: directly end negotiation if ACK ready
flat view
From: Brandon Williams <hidden>
Date: 2018-06-14 17:29:23
On 06/06, Jonathan Tan wrote:
When "ACK %s ready" is received, find_common() clears rev_list in an attempt to stop further "have" lines from being sent [1]. It is much more readable to explicitly break from the loop instead, so do this. This means that the memory in priority queue will be reclaimed only upon program exit, similar to the cases in which "ACK %s ready" is not
This seems fine for now though ideally we would remove the global priority queue and have it live on the stack somewhere in the call stack and it could be cleared unconditionally before returning.
quoted hunk ↗ jump to hunk
received. (A related problem occurs when do_fetch_pack() is invoked a second time in the same process with a possibly non-empty priority queue, but this will be solved in a subsequent patch in this patch set.) [1] The rationale is further described in the originating commit f2cba9299b ("fetch-pack: Finish negotation if remote replies "ACK %s ready"", 2011-03-14). Signed-off-by: Jonathan Tan <redacted> --- fetch-pack.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)diff --git a/fetch-pack.c b/fetch-pack.c index 2812499a5..09f5c83c4 100644 --- a/fetch-pack.c +++ b/fetch-pack.c@@ -517,10 +517,8 @@ static int find_common(struct fetch_pack_args *args, mark_common(commit, 0, 1); retval = 0; got_continue = 1; - if (ack == ACK_ready) { - clear_prio_queue(&rev_list); + if (ack == ACK_ready) got_ready = 1; - } break; } }@@ -530,6 +528,8 @@ static int find_common(struct fetch_pack_args *args, print_verbose(args, _("giving up")); break; /* give up */ } + if (got_ready) + break; } } done:@@ -1300,7 +1300,6 @@ static int process_acks(struct packet_reader *reader, struct oidset *common) } if (!strcmp(reader->line, "ready")) { - clear_prio_queue(&rev_list); received_ready = 1; continue; }-- 2.17.0.768.g1526ddbba1.dirty
-- Brandon Williams