From patchwork Wed Sep 1 13:09:50 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 12469349 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 466CEC4320E for ; Wed, 1 Sep 2021 13:09:56 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 304B861008 for ; Wed, 1 Sep 2021 13:09:56 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1344352AbhIANKv (ORCPT ); Wed, 1 Sep 2021 09:10:51 -0400 Received: from out1-smtp.messagingengine.com ([66.111.4.25]:54997 "EHLO out1-smtp.messagingengine.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1344135AbhIANKt (ORCPT ); Wed, 1 Sep 2021 09:10:49 -0400 Received: from compute3.internal (compute3.nyi.internal [10.202.2.43]) by mailout.nyi.internal (Postfix) with ESMTP id B6AAB5C01DB; Wed, 1 Sep 2021 09:09:52 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute3.internal (MEProxy); Wed, 01 Sep 2021 09:09:52 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=date :from:to:cc:subject:message-id:references:mime-version :content-type:in-reply-to; s=fm1; bh=sMga0CT9q7GLN5d1pTUK0ZuNacL OrPM0ishLT683fWM=; b=pbzKIhK0sXmI8P6FH5DNMvzcNLwDe2SZcltzhwf0Y6F iHkV4jdhTLbs7su1JywazBb1Z0IwA1Rom9b4ISUyg0PTaDjak9MbkoFj5dctvcTD /tgY9V+LAx+wkdyxBjdp9pH99qJj5Md6+aJJ86GLD9b2iXm0Dsd09mBevX9qAWuZ 9p5ixQcnq2QKIHB43JUmZZk2py6Rybw+qforiZ2uoDp8KXY3OND+KBrVY6bbpTXA 2te36woVopFEDOrzqgmkkvgw1p/NCG+DMj4cZEOiQkz7+7Pz2KxQNEnLPpfc4lwq 1i2pFaxY4WruJPWR3pE0D1ZF6nsqqQc5x0lQCmcXY/g== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to:x-me-proxy :x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s=fm3; bh=sMga0C T9q7GLN5d1pTUK0ZuNacLOrPM0ishLT683fWM=; b=MGyRvgi1znzN/A6N8UKQPj YPrG7NICLIY4Xq2nsyRPhrAOrtANM/dBmICSNdyF3DH7hxNgrayZ3cGqJ8gSojd3 dQ53pHaTzhj9elHix3XcErod15KgN1hbU3HNy0D+IEcTEg+82zC1hVk19XHuHHx+ gXG/XuGV2vPUHJ44p1uqKQNsvSjCDIaD63mly4Cr2AuVNzBNQsi9iEZ2aewkfG/X Dxes2xkNkxngyVDenmmTi4jt440LNLu2XhnibwLw5ucLsI1lOLZKkr09O5s4guO8 dnkwb7ywUPNfL2j52qBTvIiv/WDkxtN/yJicdZ7aoRdpLTdmQ10B25gQpIuZx0bw == X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvtddruddvfedgieduucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepfffhvffukfhfgggtuggjsehgtderredttdejnecuhfhrohhmpefrrghtrhhi tghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqeenucggtffrrghtthgvrh hnpeehgfejueevjeetudehgffffeffvdejfeejiedvkeffgfekuefgheevteeufeelkeen ucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehpshesph hkshdrihhm X-ME-Proxy: Received: by mail.messagingengine.com (Postfix) with ESMTPA; Wed, 1 Sep 2021 09:09:51 -0400 (EDT) Received: from localhost (ncase [10.192.0.11]) by vm-mail.pks.im (OpenSMTPD) with ESMTPSA id b8d3e360 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Wed, 1 Sep 2021 13:09:51 +0000 (UTC) Date: Wed, 1 Sep 2021 15:09:50 +0200 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Jeff King , =?iso-8859-1?q?=C6var_Arnfj=F6r=F0?= Bjarmason , Junio C Hamano , Derrick Stolee , =?iso-8859-1?q?Ren=E9?= Scharfe Subject: [PATCH v3 3/7] connected: refactor iterator to return next object ID directly Message-ID: References: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org The object ID iterator used by the connectivity checks returns the next object ID via an out-parameter and then uses a return code to indicate whether an item was found. This is a bit roundabout: instead of a separate error code, we can just return the next object ID directly and use `NULL` pointers as indicator that the iterator got no items left. Furthermore, this avoids a copy of the object ID. Refactor the iterator and all its implementations to return object IDs directly. This brings a tiny performance improvement when doing a mirror-fetch of a repository with about 2.3M refs: Benchmark #1: 328dc58b49919c43897240f2eabfa30be2ce32a4~: git-fetch Time (mean ± σ): 30.110 s ± 0.148 s [User: 27.161 s, System: 5.075 s] Range (min … max): 29.934 s … 30.406 s 10 runs Benchmark #2: 328dc58b49919c43897240f2eabfa30be2ce32a4: git-fetch Time (mean ± σ): 29.899 s ± 0.109 s [User: 26.916 s, System: 5.104 s] Range (min … max): 29.696 s … 29.996 s 10 runs Summary '328dc58b49919c43897240f2eabfa30be2ce32a4: git-fetch' ran 1.01 ± 0.01 times faster than '328dc58b49919c43897240f2eabfa30be2ce32a4~: git-fetch' While this 1% speedup could be labelled as statistically insignificant, the speedup is consistent on my machine. Furthermore, this is an end to end test, so it is expected that the improvement in the connectivity check itself is more significant. Signed-off-by: Patrick Steinhardt --- builtin/clone.c | 8 +++----- builtin/fetch.c | 7 +++---- builtin/receive-pack.c | 17 +++++++---------- connected.c | 15 ++++++++------- connected.h | 2 +- fetch-pack.c | 7 +++---- 6 files changed, 25 insertions(+), 31 deletions(-) diff --git a/builtin/clone.c b/builtin/clone.c index 66fe66679c..4a1056fcc2 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -657,7 +657,7 @@ static void write_followtags(const struct ref *refs, const char *msg) } } -static int iterate_ref_map(void *cb_data, struct object_id *oid) +static const struct object_id *iterate_ref_map(void *cb_data) { struct ref **rm = cb_data; struct ref *ref = *rm; @@ -668,13 +668,11 @@ static int iterate_ref_map(void *cb_data, struct object_id *oid) */ while (ref && !ref->peer_ref) ref = ref->next; - /* Returning -1 notes "end of list" to the caller. */ if (!ref) - return -1; + return NULL; - oidcpy(oid, &ref->old_oid); *rm = ref->next; - return 0; + return &ref->old_oid; } static void update_remote_refs(const struct ref *refs, diff --git a/builtin/fetch.c b/builtin/fetch.c index 0b18c47732..f67fe543ad 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -962,7 +962,7 @@ static int update_local_ref(struct ref *ref, } } -static int iterate_ref_map(void *cb_data, struct object_id *oid) +static const struct object_id *iterate_ref_map(void *cb_data) { struct ref **rm = cb_data; struct ref *ref = *rm; @@ -970,10 +970,9 @@ static int iterate_ref_map(void *cb_data, struct object_id *oid) while (ref && ref->status == REF_STATUS_REJECT_SHALLOW) ref = ref->next; if (!ref) - return -1; /* end of the list */ + return NULL; *rm = ref->next; - oidcpy(oid, &ref->old_oid); - return 0; + return &ref->old_oid; } struct fetch_head { diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index 2d1f97e1ca..041e915454 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -1306,7 +1306,7 @@ static void refuse_unconfigured_deny_delete_current(void) rp_error("%s", _(refuse_unconfigured_deny_delete_current_msg)); } -static int command_singleton_iterator(void *cb_data, struct object_id *oid); +static const struct object_id *command_singleton_iterator(void *cb_data); static int update_shallow_ref(struct command *cmd, struct shallow_info *si) { struct shallow_lock shallow_lock = SHALLOW_LOCK_INIT; @@ -1731,16 +1731,15 @@ static void check_aliased_updates(struct command *commands) string_list_clear(&ref_list, 0); } -static int command_singleton_iterator(void *cb_data, struct object_id *oid) +static const struct object_id *command_singleton_iterator(void *cb_data) { struct command **cmd_list = cb_data; struct command *cmd = *cmd_list; if (!cmd || is_null_oid(&cmd->new_oid)) - return -1; /* end of list */ + return NULL; *cmd_list = NULL; /* this returns only one */ - oidcpy(oid, &cmd->new_oid); - return 0; + return &cmd->new_oid; } static void set_connectivity_errors(struct command *commands, @@ -1770,7 +1769,7 @@ struct iterate_data { struct shallow_info *si; }; -static int iterate_receive_command_list(void *cb_data, struct object_id *oid) +static const struct object_id *iterate_receive_command_list(void *cb_data) { struct iterate_data *data = cb_data; struct command **cmd_list = &data->cmds; @@ -1781,13 +1780,11 @@ static int iterate_receive_command_list(void *cb_data, struct object_id *oid) /* to be checked in update_shallow_ref() */ continue; if (!is_null_oid(&cmd->new_oid) && !cmd->skip_update) { - oidcpy(oid, &cmd->new_oid); *cmd_list = cmd->next; - return 0; + return &cmd->new_oid; } } - *cmd_list = NULL; - return -1; /* end of list */ + return NULL; } static void reject_updates_to_hidden(struct command *commands) diff --git a/connected.c b/connected.c index b5f9523a5f..cf68e37a97 100644 --- a/connected.c +++ b/connected.c @@ -24,7 +24,7 @@ int check_connected(oid_iterate_fn fn, void *cb_data, struct child_process rev_list = CHILD_PROCESS_INIT; FILE *rev_list_in; struct check_connected_options defaults = CHECK_CONNECTED_INIT; - struct object_id oid; + const struct object_id *oid; int err = 0; struct packed_git *new_pack = NULL; struct transport *transport; @@ -34,7 +34,8 @@ int check_connected(oid_iterate_fn fn, void *cb_data, opt = &defaults; transport = opt->transport; - if (fn(cb_data, &oid)) { + oid = fn(cb_data); + if (!oid) { if (opt->err_fd) close(opt->err_fd); return err; @@ -73,7 +74,7 @@ int check_connected(oid_iterate_fn fn, void *cb_data, for (p = get_all_packs(the_repository); p; p = p->next) { if (!p->pack_promisor) continue; - if (find_pack_entry_one(oid.hash, p)) + if (find_pack_entry_one(oid->hash, p)) goto promisor_pack_found; } /* @@ -83,7 +84,7 @@ int check_connected(oid_iterate_fn fn, void *cb_data, goto no_promisor_pack_found; promisor_pack_found: ; - } while (!fn(cb_data, &oid)); + } while ((oid = fn(cb_data)) != NULL); return 0; } @@ -133,12 +134,12 @@ int check_connected(oid_iterate_fn fn, void *cb_data, * are sure the ref is good and not sending it to * rev-list for verification. */ - if (new_pack && find_pack_entry_one(oid.hash, new_pack)) + if (new_pack && find_pack_entry_one(oid->hash, new_pack)) continue; - if (fprintf(rev_list_in, "%s\n", oid_to_hex(&oid)) < 0) + if (fprintf(rev_list_in, "%s\n", oid_to_hex(oid)) < 0) break; - } while (!fn(cb_data, &oid)); + } while ((oid = fn(cb_data)) != NULL); if (ferror(rev_list_in) || fflush(rev_list_in)) { if (errno != EPIPE && errno != EINVAL) diff --git a/connected.h b/connected.h index 8d5a6b3ad6..6e59c92aa3 100644 --- a/connected.h +++ b/connected.h @@ -9,7 +9,7 @@ struct transport; * When called after returning the name for the last object, return -1 * to signal EOF, otherwise return 0. */ -typedef int (*oid_iterate_fn)(void *, struct object_id *oid); +typedef const struct object_id *(*oid_iterate_fn)(void *); /* * Named-arguments struct for check_connected. All arguments are diff --git a/fetch-pack.c b/fetch-pack.c index b0c7be717c..7b0e69884d 100644 --- a/fetch-pack.c +++ b/fetch-pack.c @@ -1906,16 +1906,15 @@ static void update_shallow(struct fetch_pack_args *args, oid_array_clear(&ref); } -static int iterate_ref_map(void *cb_data, struct object_id *oid) +static const struct object_id *iterate_ref_map(void *cb_data) { struct ref **rm = cb_data; struct ref *ref = *rm; if (!ref) - return -1; /* end of the list */ + return NULL; *rm = ref->next; - oidcpy(oid, &ref->old_oid); - return 0; + return &ref->old_oid; } struct ref *fetch_pack(struct fetch_pack_args *args,