From patchwork Wed Mar 12 15:56:07 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 14013656 Received: from fout-b4-smtp.messagingengine.com (fout-b4-smtp.messagingengine.com [202.12.124.147]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 8B1982566F4 for ; Wed, 12 Mar 2025 15:56:16 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=202.12.124.147 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741794978; cv=none; b=rmg8N5re4bfit+CRukRqm6VKMqBPDp9okuCligxoFU4pkf29vDSDVMWI3F5WZ4BrGHgjlEbSepyW4+Rd3Kualxp6UaQ9d1jVBn7qc4iLlY9zjOoMSNQyc75OTdwocNfO23ggqYNdSaI400X6Znm1c90L8FkN4qyK7rOGLI8PVhk= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741794978; c=relaxed/simple; bh=Luos7sQ/K/RPefsi6L+A2RlSPWFG+fvAr4lXAelps5k=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:References: In-Reply-To:To:Cc; b=VnD3IA77vStAwdS4KDXiTrAamt+NQaGq8ONgciikhHWCbxzke6xujN0bJSlCNHf7WM/V4HT43V47sS/ZuYDkAMRO784cC6CIlF5fVS6F0tETIonGprlq09oNaeRYU3XjrC5v0/duX6MLY77YLHy0ikPc5wSprDKxO81Yq14b8VY= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im; spf=pass smtp.mailfrom=pks.im; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b=F3lupS9B; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=2ID+QPGq; arc=none smtp.client-ip=202.12.124.147 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pks.im Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="F3lupS9B"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="2ID+QPGq" Received: from phl-compute-07.internal (phl-compute-07.phl.internal [10.202.2.47]) by mailfout.stl.internal (Postfix) with ESMTP id 7E0851140213; Wed, 12 Mar 2025 11:56:15 -0400 (EDT) Received: from phl-mailfrontend-01 ([10.202.2.162]) by phl-compute-07.internal (MEProxy); Wed, 12 Mar 2025 11:56:15 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :content-transfer-encoding:content-type:content-type:date:date :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to; s=fm1; t=1741794975; x=1741881375; bh=y0mNlBvGbUeoTWzlBu1AAJb4S8z2vUT7GV8pWSe2ASA=; b= F3lupS9Bi4f0vmdRpYSFbpJOo42MG896WG3Ka5LHaoQ7LPoF4c5/K9opX9OvSmqV WLJ3UUBPrKj71hZjzzYjtdusVIfKG/Uc/+TvJRuq8GGQSZMhKOi4xNiuf+f72ONu RZ83xRU6OQRVg0s353Ngh3Q5p2K0bQvYZWV/K41Xi3R+FzR/f7ExSWeLjeK58Tfj olK8+7SsYfW8rInQkcyCMkqBtmzIACAaEcJqBFq2QW0mgHbOkWzrAa8o9rRFc9kX kGTbrpItVH/QtQNKDtfN9ZHQeKHSIyAR/0f+17KYeFj3TF+8l5hxaZxpmYPhNqhq T5tfq4OH5Gcw5gl+dwwkgg== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:content-type:date:date:feedback-id:feedback-id :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to:x-me-proxy :x-me-sender:x-me-sender:x-sasl-enc; s=fm1; t=1741794975; x= 1741881375; bh=y0mNlBvGbUeoTWzlBu1AAJb4S8z2vUT7GV8pWSe2ASA=; b=2 ID+QPGqviBs4z0by1uu8UJjWNW0/2fsh44pOVUfWOldYsBeHlbC6a+ZmcmRgWzbH +kbKp33ZD/AzPGntgW1BBbVQmreux9HEcgFegz0Km6OT2xADf9tRck0QK6ekBNpt CK95EztAwT3E9JNM7jjwXo3fPQMUQis9CdVImplAjEX2a3CU5O0JVs2ab8/U6gq8 LezaXlyiRA4/gBk/sSozdHs6ns252bYRCIpusODqLf1ix34n8veNZJOWFUvOf/lp l0Tegr+mXHiS6L8ZcgKbMD14nnSy5UPtP+jn/U3sxG9AueRgifUZb/yrGZqBdPan SjhLwtozZOpS/HvL+kziQ== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeefvddrtddtgdduvdehgeekucetufdoteggodetrf dotffvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdggtfgfnhhsuhgsshgtrhhisggv pdfurfetoffkrfgpnffqhgenuceurghilhhouhhtmecufedttdenucesvcftvggtihhpih gvnhhtshculddquddttddmnecujfgurhephfffufggtgfgkfhfjgfvvefosehtjeertder tdejnecuhfhrohhmpefrrghtrhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhksh drihhmqeenucggtffrrghtthgvrhhnpeffueeiudejvdekheeuvdekfeffiedvueelteek udehjeetkeegvddugfdtgfeileenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmh epmhgrihhlfhhrohhmpehpshesphhkshdrihhmpdhnsggprhgtphhtthhopeejpdhmohgu vgepshhmthhpohhuthdprhgtphhtthhopehpvghffhesphgvfhhfrdhnvghtpdhrtghpth htohepghhithesvhhgvghrrdhkvghrnhgvlhdrohhrghdprhgtphhtthhopehshhgvjhhi rghluhhosehgmhgrihhlrdgtohhmpdhrtghpthhtohepghhithhsthgvrhesphhosghogi drtghomhdprhgtphhtthhopehsrghnuggrlhhssegtrhhushhthihtohhothhhphgrshht vgdrnhgvthdprhgtphhtthhopehkrghrthhhihhkrddukeeksehgmhgrihhlrdgtohhmpd hrtghpthhtoheptghhrhhishgtohholhesthhugihfrghmihhlhidrohhrgh X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Wed, 12 Mar 2025 11:56:13 -0400 (EDT) Received: by vm-mail (OpenSMTPD) with ESMTPSA id a7dd6985 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Wed, 12 Mar 2025 15:56:11 +0000 (UTC) From: Patrick Steinhardt Date: Wed, 12 Mar 2025 16:56:07 +0100 Subject: [PATCH v6 01/16] object-name: introduce `repo_get_oid_with_flags()` Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Message-Id: <20250312-pks-update-ref-optimization-v6-1-f778e0414f55@pks.im> References: <20250312-pks-update-ref-optimization-v6-0-f778e0414f55@pks.im> In-Reply-To: <20250312-pks-update-ref-optimization-v6-0-f778e0414f55@pks.im> To: git@vger.kernel.org Cc: Karthik Nayak , "brian m. carlson" , Jeff King , Junio C Hamano , shejialuo , Christian Couder X-Mailer: b4 0.14.2 Introduce a new function `repo_get_oid_with_flags()`. This function behaves the same as `repo_get_oid()`, except that it takes an extra `flags` parameter that it ends up passing to `get_oid_with_context()`. This function will be used in a subsequent commit. Signed-off-by: Patrick Steinhardt --- object-name.c | 14 ++++++++------ object-name.h | 6 ++++++ 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/object-name.c b/object-name.c index 945d5bdef25..233f3f861e3 100644 --- a/object-name.c +++ b/object-name.c @@ -1794,18 +1794,20 @@ void object_context_release(struct object_context *ctx) strbuf_release(&ctx->symlink_path); } -/* - * This is like "get_oid_basic()", except it allows "object ID expressions", - * notably "xyz^" for "parent of xyz" - */ -int repo_get_oid(struct repository *r, const char *name, struct object_id *oid) +int repo_get_oid_with_flags(struct repository *r, const char *name, + struct object_id *oid, unsigned flags) { struct object_context unused; - int ret = get_oid_with_context(r, name, 0, oid, &unused); + int ret = get_oid_with_context(r, name, flags, oid, &unused); object_context_release(&unused); return ret; } +int repo_get_oid(struct repository *r, const char *name, struct object_id *oid) +{ + return repo_get_oid_with_flags(r, name, oid, 0); +} + /* * This returns a non-zero value if the string (built using printf * format and the given arguments) is not a valid object. diff --git a/object-name.h b/object-name.h index 8dba4a47a47..cda4934cd5f 100644 --- a/object-name.h +++ b/object-name.h @@ -51,6 +51,12 @@ void strbuf_repo_add_unique_abbrev(struct strbuf *sb, struct repository *repo, void strbuf_add_unique_abbrev(struct strbuf *sb, const struct object_id *oid, int abbrev_len); +/* + * This is like "get_oid_basic()", except it allows "object ID expressions", + * notably "xyz^" for "parent of xyz". Accepts GET_OID_* flags. + */ +int repo_get_oid_with_flags(struct repository *r, const char *str, + struct object_id *oid, unsigned flags); int repo_get_oid(struct repository *r, const char *str, struct object_id *oid); __attribute__((format (printf, 2, 3))) int get_oidf(struct object_id *oid, const char *fmt, ...); From patchwork Wed Mar 12 15:56:08 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 14013657 Received: from fhigh-b4-smtp.messagingengine.com (fhigh-b4-smtp.messagingengine.com [202.12.124.155]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 8B780256C7F for ; Wed, 12 Mar 2025 15:56:16 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=202.12.124.155 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741794979; cv=none; b=goP2NjXqEHT93987hfk0G3F8ecs/wQJdnmYgR3WItxZsL121fu0q9FgRms+YMBDHfDehv/NGwGyMeKtqopeYkferEN70lKVRnjVsFSchPskuvsTdJOWeGMgimCfYhai/DOMjKd1AHHRaYnZz3dyVhveh9ZWP6+CtbR50/GIGKIw= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741794979; c=relaxed/simple; bh=0f9Y9/EZ53iuqSwxWV5WZcS9qblTqOcD8PlnTpWtKL0=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:References: In-Reply-To:To:Cc; b=cFTjbXqjlYVj6B+GXmn0EDq3mhnxMi3hABudioz3aoCZRE4JobRkqqP2ma66kyz34zDVEuxJdDzzXE53RBVS7YfTjD/KEAr0j3V3DI4HRbEo8ewZk5S79/eW1gqxuTfanWCkgnqAmYZN1fH3V5uJlOwX84puPXk4HZbAMfJy5Ck= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im; spf=pass smtp.mailfrom=pks.im; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b=C6pwJGSr; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=2XgK6T08; arc=none smtp.client-ip=202.12.124.155 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pks.im Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="C6pwJGSr"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="2XgK6T08" Received: from phl-compute-09.internal (phl-compute-09.phl.internal [10.202.2.49]) by mailfhigh.stl.internal (Postfix) with ESMTP id 83482254030B; Wed, 12 Mar 2025 11:56:15 -0400 (EDT) Received: from phl-mailfrontend-02 ([10.202.2.163]) by phl-compute-09.internal (MEProxy); Wed, 12 Mar 2025 11:56:15 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :content-transfer-encoding:content-type:content-type:date:date :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to; s=fm1; t=1741794975; x=1741881375; bh=Odo4sGjB8UB7AFyG+XVCax6uy/MF29PSaSNDMmuBU5M=; b= C6pwJGSryDcuUO1Y95uoY3TNre2BUiv3JNAPWHgArMXT7fmWbi0rcRBMsSOfXNUy L7k4Cbak2P59rG+FZjJROqDAEoGQ4a4OHI3wa8LDxQ4KDeskDLLJtLPthotBdabL lI9G7DCcyQP8yME3bDImw5NO37lrgVoZL6+70k9Lfzr7Uh9LuOP9RYRAjid1oFYj P19pCHS/A2jJBaF+urkNzLgPE8IRnEVOjP+YSWimSMJkgj9k1FI3CHRuzlg34nVl A993skSr/CXG0DT9tiIfcz8r+1EzepPn5l9GfJphBAgCWnWFeUWxpyYwXVk77mb6 jxsO4Ac5VJk67cWHC4bK3A== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:content-type:date:date:feedback-id:feedback-id :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to:x-me-proxy :x-me-sender:x-me-sender:x-sasl-enc; s=fm1; t=1741794975; x= 1741881375; bh=Odo4sGjB8UB7AFyG+XVCax6uy/MF29PSaSNDMmuBU5M=; b=2 XgK6T08bNLC7DhxYuun5oZ4u9ONeUc/Qn2oDXbNKj+DRP0L4BSw1R80WDHjzIsBB 2M69/vzxC+ZQxWWOHVnUfmGKapSW3rJIOCQrkImmXhHTm6PQcm5STms9CQKaWP70 Zjh5aHYyCQWwUtmPBvJ3QC02xOWN1H+E9/vitC8pdnEIid0XEkvfobtlVutgyvk/ gMI3w5KLRdZg2v4lXm0uFdsk2sDRto9hNmLRcIeQONv5UyDaqrbkQENjRI31StWr YLRQFOlTVc8b7UIlzhsfQXrmSlIeXlCa4DrPkep6Hz6+WE30fV3AWQGjOXfLB/bA Ecfy33MLlnSkw85rphmSw== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeefvddrtddtgdduvdehgeelucetufdoteggodetrf dotffvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdggtfgfnhhsuhgsshgtrhhisggv pdfurfetoffkrfgpnffqhgenuceurghilhhouhhtmecufedttdenucesvcftvggtihhpih gvnhhtshculddquddttddmnecujfgurhephfffufggtgfgkfhfjgfvvefosehtjeertder tdejnecuhfhrohhmpefrrghtrhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhksh drihhmqeenucggtffrrghtthgvrhhnpeffueeiudejvdekheeuvdekfeffiedvueelteek udehjeetkeegvddugfdtgfeileenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmh epmhgrihhlfhhrohhmpehpshesphhkshdrihhmpdhnsggprhgtphhtthhopeejpdhmohgu vgepshhmthhpohhuthdprhgtphhtthhopehkrghrthhhihhkrddukeeksehgmhgrihhlrd gtohhmpdhrtghpthhtohepphgvfhhfsehpvghffhdrnhgvthdprhgtphhtthhopegthhhr ihhstghoohhlsehtuhigfhgrmhhilhihrdhorhhgpdhrtghpthhtohepghhithesvhhgvg hrrdhkvghrnhgvlhdrohhrghdprhgtphhtthhopehsrghnuggrlhhssegtrhhushhthiht ohhothhhphgrshhtvgdrnhgvthdprhgtphhtthhopehgihhtshhtvghrsehpohgsohigrd gtohhmpdhrtghpthhtohepshhhvghjihgrlhhuohesghhmrghilhdrtghomh X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Wed, 12 Mar 2025 11:56:13 -0400 (EDT) Received: by vm-mail (OpenSMTPD) with ESMTPSA id bb8b8ba0 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Wed, 12 Mar 2025 15:56:12 +0000 (UTC) From: Patrick Steinhardt Date: Wed, 12 Mar 2025 16:56:08 +0100 Subject: [PATCH v6 02/16] object-name: allow skipping ambiguity checks in `get_oid()` family Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Message-Id: <20250312-pks-update-ref-optimization-v6-2-f778e0414f55@pks.im> References: <20250312-pks-update-ref-optimization-v6-0-f778e0414f55@pks.im> In-Reply-To: <20250312-pks-update-ref-optimization-v6-0-f778e0414f55@pks.im> To: git@vger.kernel.org Cc: Karthik Nayak , "brian m. carlson" , Jeff King , Junio C Hamano , shejialuo , Christian Couder X-Mailer: b4 0.14.2 When reading an object ID via `get_oid_basic()` or any of its related functions we perform a check whether the object ID is ambiguous, which can be the case when a reference with the same name exists. While the check is generally helpful, there are cases where it only adds to the runtime overhead without providing much of a benefit. Add a new flag that allows us to disable the check. The flag will be used in a subsequent commit. Signed-off-by: Patrick Steinhardt --- hash.h | 23 ++++++++++++----------- object-name.c | 4 +++- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/hash.h b/hash.h index 4367acfec50..5e3c462dc5e 100644 --- a/hash.h +++ b/hash.h @@ -193,17 +193,18 @@ struct object_id { int algo; /* XXX requires 4-byte alignment */ }; -#define GET_OID_QUIETLY 01 -#define GET_OID_COMMIT 02 -#define GET_OID_COMMITTISH 04 -#define GET_OID_TREE 010 -#define GET_OID_TREEISH 020 -#define GET_OID_BLOB 040 -#define GET_OID_FOLLOW_SYMLINKS 0100 -#define GET_OID_RECORD_PATH 0200 -#define GET_OID_ONLY_TO_DIE 04000 -#define GET_OID_REQUIRE_PATH 010000 -#define GET_OID_HASH_ANY 020000 +#define GET_OID_QUIETLY 01 +#define GET_OID_COMMIT 02 +#define GET_OID_COMMITTISH 04 +#define GET_OID_TREE 010 +#define GET_OID_TREEISH 020 +#define GET_OID_BLOB 040 +#define GET_OID_FOLLOW_SYMLINKS 0100 +#define GET_OID_RECORD_PATH 0200 +#define GET_OID_ONLY_TO_DIE 04000 +#define GET_OID_REQUIRE_PATH 010000 +#define GET_OID_HASH_ANY 020000 +#define GET_OID_SKIP_AMBIGUITY_CHECK 040000 #define GET_OID_DISAMBIGUATORS \ (GET_OID_COMMIT | GET_OID_COMMITTISH | \ diff --git a/object-name.c b/object-name.c index 233f3f861e3..85444dbb15b 100644 --- a/object-name.c +++ b/object-name.c @@ -961,7 +961,9 @@ static int get_oid_basic(struct repository *r, const char *str, int len, int fatal = !(flags & GET_OID_QUIETLY); if (len == r->hash_algo->hexsz && !get_oid_hex(str, oid)) { - if (repo_settings_get_warn_ambiguous_refs(r) && warn_on_object_refname_ambiguity) { + if (!(flags & GET_OID_SKIP_AMBIGUITY_CHECK) && + repo_settings_get_warn_ambiguous_refs(r) && + warn_on_object_refname_ambiguity) { refs_found = repo_dwim_ref(r, str, len, &tmp_oid, &real_ref, 0); if (refs_found > 0) { warning(warn_msg, len, str); From patchwork Wed Mar 12 15:56:09 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 14013658 Received: from fhigh-b4-smtp.messagingengine.com (fhigh-b4-smtp.messagingengine.com [202.12.124.155]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 8E4CC2571A6 for ; Wed, 12 Mar 2025 15:56:18 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=202.12.124.155 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741794980; cv=none; b=VD9KiPJZYVVrQ8wFEMM0EfjTPEtJpjW53m43une9IqyFz7tKZFNKZIza8AZf6eozAfb+MUWeImbopLeYVF7xnZqhtTCsMU1ldCx7AjNqOoh+5CGBvKvQ8J1J9xgSmfUyoUztCXIUNiBhKNIphle/FAQDrvFDxntA8Hwh3tlOtMM= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741794980; c=relaxed/simple; bh=UEQlJe7aVyXCfgy8qDCNY20ApVIz/18SbwSHOrhd9dM=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:References: In-Reply-To:To:Cc; b=GuDGQSqzqlc9/wAEitQewRz8pK9CrNiulHTc0w6ReyfhzisxVs9JAal0dSYGx4xh+GAhOVseuBMUSnv2YChtZE0xGQOMwAiqe1m8AXX/mTL2pSVtGRgwFsIKBJoxeX7S3VGlf/A8i8pVqidzJLJ9QG/+cmiOpq4N/n48YLmtCxA= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im; spf=pass smtp.mailfrom=pks.im; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b=NCrDVEz4; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=YSvMnSfX; arc=none smtp.client-ip=202.12.124.155 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pks.im Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="NCrDVEz4"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="YSvMnSfX" Received: from phl-compute-13.internal (phl-compute-13.phl.internal [10.202.2.53]) by mailfhigh.stl.internal (Postfix) with ESMTP id 787C52540312; Wed, 12 Mar 2025 11:56:17 -0400 (EDT) Received: from phl-mailfrontend-01 ([10.202.2.162]) by phl-compute-13.internal (MEProxy); Wed, 12 Mar 2025 11:56:17 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :content-transfer-encoding:content-type:content-type:date:date :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to; s=fm1; t=1741794977; x=1741881377; bh=IwM6MUJPchaH0pGEK87waKftLh0xe4LSG+oJ2KQbL9E=; b= NCrDVEz4z5TeYTbwWZZwtH9feKmXa21vqNo/lfl36MDL26fDtlbbLs+RaO/r36Ht tT+VurPMVDj8WPnMiSJGu6VZgGi7+Pn9irULpFb0fLi2zoZlNlAg12KBV6ypiyf4 C8PFQ4YXturT4FSn32azVN2dicBNK4JhlUZG+tEAevgEGJkOrCGsE2GkmVZiGOdy CAP4QtUoFZCPSEX8Z3ZKgskyK4wJdOwqAN/5CXpUwzAlAWg98lsLMTMNeRYwpuR5 FIKfCVOm7jrCgAf+L8LpxuSBiWyb9XgOWjqfbAtNAJOOV9vxyT5aBp32BDQLCapG xwJfWdZ1vxS+qjaz/qbcsw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:content-type:date:date:feedback-id:feedback-id :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to:x-me-proxy :x-me-sender:x-me-sender:x-sasl-enc; s=fm1; t=1741794977; x= 1741881377; bh=IwM6MUJPchaH0pGEK87waKftLh0xe4LSG+oJ2KQbL9E=; b=Y SvMnSfX7mu/L1k5cd9fC6OMG6NblhUtc3TFV4EV8JDx4PgqBy/CiaSGfaE1y9cvL /lJPmN/wVojtlvzacgoocOF0Sua3yMW48e5lapXTANIG/jngqHziAA/sc1A4B4ZS 3ChfELGsttSCCC7+5PBDQmOdPBRbhxuNeLnPgzxJ/ev4+RtoHzoHtLCO31wiQToB fetNmxXjK+Ob1w1ccQLy9qIjACERdMzhlhUzP6CSrVN6SmXA0pCdfXDcT+sIlVJr v69fdwBAXE+T8+aLb908eiJJ18wFh3AIi+BIZM4AMK9daCU6N2rRGBxcsmYkTqcU n63V3zxN94tLXlEtKUgUQ== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeefvddrtddtgdduvdehgeekucetufdoteggodetrf dotffvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdggtfgfnhhsuhgsshgtrhhisggv pdfurfetoffkrfgpnffqhgenuceurghilhhouhhtmecufedttdenucesvcftvggtihhpih gvnhhtshculddquddttddmnecujfgurhephfffufggtgfgkfhfjgfvvefosehtkeertder tdejnecuhfhrohhmpefrrghtrhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhksh drihhmqeenucggtffrrghtthgvrhhnpeefhfeugeelheefjeektdffhedvhfdvteefgfdt udffudevveetgeeuuedtkefhgeenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmh epmhgrihhlfhhrohhmpehpshesphhkshdrihhmpdhnsggprhgtphhtthhopeejpdhmohgu vgepshhmthhpohhuthdprhgtphhtthhopehgihhtsehvghgvrhdrkhgvrhhnvghlrdhorh hgpdhrtghpthhtohepshgrnhgurghlshestghruhhsthihthhoohhthhhprghsthgvrdhn vghtpdhrtghpthhtoheptghhrhhishgtohholhesthhugihfrghmihhlhidrohhrghdprh gtphhtthhopehkrghrthhhihhkrddukeeksehgmhgrihhlrdgtohhmpdhrtghpthhtohep shhhvghjihgrlhhuohesghhmrghilhdrtghomhdprhgtphhtthhopehpvghffhesphgvfh hfrdhnvghtpdhrtghpthhtohepghhithhsthgvrhesphhosghogidrtghomh X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Wed, 12 Mar 2025 11:56:15 -0400 (EDT) Received: by vm-mail (OpenSMTPD) with ESMTPSA id c5def754 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Wed, 12 Mar 2025 15:56:13 +0000 (UTC) From: Patrick Steinhardt Date: Wed, 12 Mar 2025 16:56:09 +0100 Subject: [PATCH v6 03/16] builtin/update-ref: skip ambiguity checks when parsing object IDs Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Message-Id: <20250312-pks-update-ref-optimization-v6-3-f778e0414f55@pks.im> References: <20250312-pks-update-ref-optimization-v6-0-f778e0414f55@pks.im> In-Reply-To: <20250312-pks-update-ref-optimization-v6-0-f778e0414f55@pks.im> To: git@vger.kernel.org Cc: Karthik Nayak , "brian m. carlson" , Jeff King , Junio C Hamano , shejialuo , Christian Couder X-Mailer: b4 0.14.2 Most of the commands in git-update-ref(1) accept an old and/or new object ID to update a specific reference to. These object IDs get parsed via `repo_get_oid()`, which not only handles plain object IDs, but also those that have a suffix like "~" or "^2". More surprisingly though, it even knows to resolve arbitrary revisions, despite the fact that its manpage does not mention this fact even once. One consequence of this is that we also check for ambiguous references: when parsing a full object ID where the DWIM mechanism would also cause us to resolve it as a branch, we'd end up printing a warning. While this check makes sense to have in general, it is arguably less useful in the context of git-update-ref(1). This is due to multiple reasons: - The manpage is explicitly structured around object IDs. So if we see a fully blown object ID, the intent should be quite clear in general. - The command is part of our plumbing layer and not a tool that users would generally use in interactive workflows. As such, the warning will likely not be visible to anybody in the first place. - Users can and should use the fully-qualified refname in case there is any potential for ambiguity. And given that this command is part of our plumbing layer, one should always try to be as defensive as possible and use fully-qualified refnames. Furthermore, this check can be quite expensive when updating lots of references via `--stdin`, because we try to read multiple references per object ID that we parse according to the DWIM rules. This effect can be seen both with the "files" and "reftable" backend. The issue is not unique to git-update-ref(1), but was also an issue in git-cat-file(1), where it was addressed by disabling the ambiguity check in 25fba78d36b (cat-file: disable object/refname ambiguity check for batch mode, 2013-07-12). Disable the warning in git-update-ref(1), which provides a significant speedup with both backends. The user-visible outcome is unchanged even when ambiguity exists, except that we don't show the warning anymore. The following benchmark creates 10000 new references with a 100000 preexisting refs with the "files" backend: Benchmark 1: update-ref: create many refs (refformat = files, preexisting = 100000, new = 10000, revision = HEAD~) Time (mean ± σ): 467.3 ms ± 5.1 ms [User: 100.0 ms, System: 365.1 ms] Range (min … max): 461.9 ms … 479.3 ms 10 runs Benchmark 2: update-ref: create many refs (refformat = files, preexisting = 100000, new = 10000, revision = HEAD) Time (mean ± σ): 394.1 ms ± 5.8 ms [User: 63.3 ms, System: 327.6 ms] Range (min … max): 384.9 ms … 405.7 ms 10 runs Summary update-ref: create many refs (refformat = files, preexisting = 100000, new = 10000, revision = HEAD) ran 1.19 ± 0.02 times faster than update-ref: create many refs (refformat = files, preexisting = 100000, new = 10000, revision = HEAD~) And with the "reftable" backend: Benchmark 1: update-ref: create many refs (refformat = reftable, preexisting = 100000, new = 10000, revision = HEAD~) Time (mean ± σ): 146.9 ms ± 2.2 ms [User: 90.4 ms, System: 56.0 ms] Range (min … max): 142.7 ms … 150.8 ms 19 runs Benchmark 2: update-ref: create many refs (refformat = reftable, preexisting = 100000, new = 10000, revision = HEAD) Time (mean ± σ): 63.2 ms ± 1.1 ms [User: 41.0 ms, System: 21.8 ms] Range (min … max): 61.1 ms … 66.6 ms 41 runs Summary update-ref: create many refs (refformat = reftable, preexisting = 100000, new = 10000, revision = HEAD) ran 2.32 ± 0.05 times faster than update-ref: create many refs (refformat = reftable, preexisting = 100000, new = 10000, revision = HEAD~) Note that the absolute improvement with both backends is roughly in the same ballpark, but the relative improvement for the "reftable" backend is more significant because writing the new table to disk is faster in the first place. Signed-off-by: Patrick Steinhardt --- builtin/update-ref.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/builtin/update-ref.c b/builtin/update-ref.c index 4d35bdc4b4b..1d541e13ade 100644 --- a/builtin/update-ref.c +++ b/builtin/update-ref.c @@ -179,7 +179,8 @@ static int parse_next_oid(const char **next, const char *end, (*next)++; *next = parse_arg(*next, &arg); if (arg.len) { - if (repo_get_oid(the_repository, arg.buf, oid)) + if (repo_get_oid_with_flags(the_repository, arg.buf, oid, + GET_OID_SKIP_AMBIGUITY_CHECK)) goto invalid; } else { /* Without -z, an empty value means all zeros: */ @@ -197,7 +198,8 @@ static int parse_next_oid(const char **next, const char *end, *next += arg.len; if (arg.len) { - if (repo_get_oid(the_repository, arg.buf, oid)) + if (repo_get_oid_with_flags(the_repository, arg.buf, oid, + GET_OID_SKIP_AMBIGUITY_CHECK)) goto invalid; } else if (flags & PARSE_SHA1_ALLOW_EMPTY) { /* With -z, treat an empty value as all zeros: */ @@ -299,7 +301,8 @@ static void parse_cmd_symref_update(struct ref_transaction *transaction, die("symref-update %s: expected old value", refname); if (!strcmp(old_arg, "oid")) { - if (repo_get_oid(the_repository, old_target, &old_oid)) + if (repo_get_oid_with_flags(the_repository, old_target, &old_oid, + GET_OID_SKIP_AMBIGUITY_CHECK)) die("symref-update %s: invalid oid: %s", refname, old_target); have_old_oid = 1; @@ -772,7 +775,8 @@ int cmd_update_ref(int argc, refname = argv[0]; value = argv[1]; oldval = argv[2]; - if (repo_get_oid(the_repository, value, &oid)) + if (repo_get_oid_with_flags(the_repository, value, &oid, + GET_OID_SKIP_AMBIGUITY_CHECK)) die("%s: not a valid SHA1", value); } @@ -783,7 +787,8 @@ int cmd_update_ref(int argc, * must not already exist: */ oidclr(&oldoid, the_repository->hash_algo); - else if (repo_get_oid(the_repository, oldval, &oldoid)) + else if (repo_get_oid_with_flags(the_repository, oldval, &oldoid, + GET_OID_SKIP_AMBIGUITY_CHECK)) die("%s: not a valid old SHA1", oldval); } From patchwork Wed Mar 12 15:56:10 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 14013659 Received: from fout-b4-smtp.messagingengine.com (fout-b4-smtp.messagingengine.com [202.12.124.147]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 930B92571C4 for ; Wed, 12 Mar 2025 15:56:18 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=202.12.124.147 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741794980; cv=none; b=Kc55+BJqll2HFzqyNUPVNxI197FCaz7FJ/s1sidqhFiTdkE9KWrISd7fiUHCZ+IqvzGglJ9v/LrwN06SbzE+/fg9L+4oXRZSHT6N/gAAfjdunOIjVeHcE9/G8/jrYyGL/Wr9642VVtazPAXeoH2afYSh8iWKKnEptG0/CmG4oT4= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741794980; c=relaxed/simple; bh=3kdGZPL2ENDTBmC7UevJ12/4eqDbZsY1F0jafitTZl8=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:References: In-Reply-To:To:Cc; b=ZWMrlhqXvg/KwvU2i8tzZN26eWIvJdIhht3dLrV0olNcmDd7zKWoF8H3p2GnVUu8sdZ8rp9Sp5rDwBEG3LkqpysSMMdJCP8VQyIKV2PQ87mRNHCP2ywdLP4onyKdQUkdIqS9HNh7wVai4SHIWtMJHbxdiQpOzTXa9o9DG56Fr90= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im; spf=pass smtp.mailfrom=pks.im; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b=eeVx3YdF; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=TqslUMxi; arc=none smtp.client-ip=202.12.124.147 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pks.im Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="eeVx3YdF"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="TqslUMxi" Received: from phl-compute-07.internal (phl-compute-07.phl.internal [10.202.2.47]) by mailfout.stl.internal (Postfix) with ESMTP id 8B32A11401E3; Wed, 12 Mar 2025 11:56:17 -0400 (EDT) Received: from phl-mailfrontend-02 ([10.202.2.163]) by phl-compute-07.internal (MEProxy); Wed, 12 Mar 2025 11:56:17 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :content-transfer-encoding:content-type:content-type:date:date :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to; s=fm1; t=1741794977; x=1741881377; bh=KM0W6Noa1jrB8Rui3gkcxBbCxVhqgDyEONEyXs+tsPs=; b= eeVx3YdFo5PjEnoLueBLSQKzPuaYEuGNM9ifQYAaMnbS1QOcovJqrkMKeKXsSNFA NRqT3mIfbO4vXOULzOYxXEsGJGo+7anbQN+c7tHuGVO02Jo4pzqixjNT+zfDv8hI OxNr6m25MZAurZX+M4P+Z/RUey7ZrEh+sssxiQO+k8t1B9pMXWFNdEOV0x0S7LXL RLX2KDyiFgCtn6yItVHtWpI3p0RaFoQtOPgJJjnfMoWk5mqj3C7Qy/M9WFP8qPoE hmKYWsM48Yqqn66oyNQ5CYCLx2PfQCXQwD3a07ijNyC+waMmOxdf421fO+Qopmp5 s1iX+TVbNi2lA6oRYBnArw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:content-type:date:date:feedback-id:feedback-id :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to:x-me-proxy :x-me-sender:x-me-sender:x-sasl-enc; s=fm1; t=1741794977; x= 1741881377; bh=KM0W6Noa1jrB8Rui3gkcxBbCxVhqgDyEONEyXs+tsPs=; b=T qslUMxiG/NfxfXf8ER3YJn6W1d7xylVC0MqxrNRypveYXxItH65bokpOJvnYqL9O yRKUMCOblf0DmT3oHrB8r4YPP+pDKCkyjqyGhdt7KVbuUk9jcdppKVuTxSUrXbTE sI9LZfCRb+jy0tn09Qn44Cc71fVMCbkbiIw/R+Y6aG67FJQBHB+pw8H1iEXHAK5P ISZY3NGIyqZ+hBQDaH12Kecr8h0me/Rgqi5/5QQNGLWmXd26rD+FkMQZaP1sA5QO KOTNU6OUWRVMBw/qBUplpxqkbzbdA79XKZ7GvyGyMl+noQwZt1/FnPu+0DBzNcit jjsOSoJgeABTxFRJnUKIw== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeefvddrtddtgdduvdehgeekucetufdoteggodetrf dotffvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdggtfgfnhhsuhgsshgtrhhisggv pdfurfetoffkrfgpnffqhgenuceurghilhhouhhtmecufedttdenucesvcftvggtihhpih gvnhhtshculddquddttddmnecujfgurhephfffufggtgfgkfhfjgfvvefosehtjeertder tdejnecuhfhrohhmpefrrghtrhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhksh drihhmqeenucggtffrrghtthgvrhhnpeffueeiudejvdekheeuvdekfeffiedvueelteek udehjeetkeegvddugfdtgfeileenucevlhhushhtvghrufhiiigvpedunecurfgrrhgrmh epmhgrihhlfhhrohhmpehpshesphhkshdrihhmpdhnsggprhgtphhtthhopeejpdhmohgu vgepshhmthhpohhuthdprhgtphhtthhopehpvghffhesphgvfhhfrdhnvghtpdhrtghpth htohepshhhvghjihgrlhhuohesghhmrghilhdrtghomhdprhgtphhtthhopehgihhtsehv ghgvrhdrkhgvrhhnvghlrdhorhhgpdhrtghpthhtoheptghhrhhishgtohholhesthhugi hfrghmihhlhidrohhrghdprhgtphhtthhopehkrghrthhhihhkrddukeeksehgmhgrihhl rdgtohhmpdhrtghpthhtohepghhithhsthgvrhesphhosghogidrtghomhdprhgtphhtth hopehsrghnuggrlhhssegtrhhushhthihtohhothhhphgrshhtvgdrnhgvth X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Wed, 12 Mar 2025 11:56:15 -0400 (EDT) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 79240a2b (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Wed, 12 Mar 2025 15:56:13 +0000 (UTC) From: Patrick Steinhardt Date: Wed, 12 Mar 2025 16:56:10 +0100 Subject: [PATCH v6 04/16] refs: introduce function to batch refname availability checks Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Message-Id: <20250312-pks-update-ref-optimization-v6-4-f778e0414f55@pks.im> References: <20250312-pks-update-ref-optimization-v6-0-f778e0414f55@pks.im> In-Reply-To: <20250312-pks-update-ref-optimization-v6-0-f778e0414f55@pks.im> To: git@vger.kernel.org Cc: Karthik Nayak , "brian m. carlson" , Jeff King , Junio C Hamano , shejialuo , Christian Couder X-Mailer: b4 0.14.2 The `refs_verify_refname_available()` functions checks whether a reference update can be committed or whether it would conflict with either a prefix or suffix thereof. This function needs to be called once per reference that one wants to check, which requires us to redo a couple of checks every time the function is called. Introduce a new function `refs_verify_refnames_available()` that does the same, but for a list of references. For now, the new function uses the exact same implementation, except that we loop through all refnames provided by the caller. This will be tuned in subsequent commits. The existing `refs_verify_refname_available()` function is reimplemented on top of the new function. As such, the diff is best viewed with the `--ignore-space-change option`. Signed-off-by: Patrick Steinhardt --- refs.c | 170 +++++++++++++++++++++++++++++++++++++---------------------------- refs.h | 12 +++++ 2 files changed, 110 insertions(+), 72 deletions(-) diff --git a/refs.c b/refs.c index f4094a326a9..d91a2184e06 100644 --- a/refs.c +++ b/refs.c @@ -2467,19 +2467,16 @@ int ref_transaction_commit(struct ref_transaction *transaction, return ret; } -int refs_verify_refname_available(struct ref_store *refs, - const char *refname, - const struct string_list *extras, - const struct string_list *skip, - unsigned int initial_transaction, - struct strbuf *err) +int refs_verify_refnames_available(struct ref_store *refs, + const struct string_list *refnames, + const struct string_list *extras, + const struct string_list *skip, + unsigned int initial_transaction, + struct strbuf *err) { - const char *slash; - const char *extra_refname; struct strbuf dirname = STRBUF_INIT; struct strbuf referent = STRBUF_INIT; - struct object_id oid; - unsigned int type; + struct string_list_item *item; int ret = -1; /* @@ -2489,79 +2486,91 @@ int refs_verify_refname_available(struct ref_store *refs, assert(err); - strbuf_grow(&dirname, strlen(refname) + 1); - for (slash = strchr(refname, '/'); slash; slash = strchr(slash + 1, '/')) { - /* - * Just saying "Is a directory" when we e.g. can't - * lock some multi-level ref isn't very informative, - * the user won't be told *what* is a directory, so - * let's not use strerror() below. - */ - int ignore_errno; - /* Expand dirname to the new prefix, not including the trailing slash: */ - strbuf_add(&dirname, refname + dirname.len, slash - refname - dirname.len); + for_each_string_list_item(item, refnames) { + const char *refname = item->string; + const char *extra_refname; + struct object_id oid; + unsigned int type; + const char *slash; + + strbuf_reset(&dirname); + + for (slash = strchr(refname, '/'); slash; slash = strchr(slash + 1, '/')) { + /* + * Just saying "Is a directory" when we e.g. can't + * lock some multi-level ref isn't very informative, + * the user won't be told *what* is a directory, so + * let's not use strerror() below. + */ + int ignore_errno; + + /* Expand dirname to the new prefix, not including the trailing slash: */ + strbuf_add(&dirname, refname + dirname.len, slash - refname - dirname.len); + + /* + * We are still at a leading dir of the refname (e.g., + * "refs/foo"; if there is a reference with that name, + * it is a conflict, *unless* it is in skip. + */ + if (skip && string_list_has_string(skip, dirname.buf)) + continue; + + if (!initial_transaction && + !refs_read_raw_ref(refs, dirname.buf, &oid, &referent, + &type, &ignore_errno)) { + strbuf_addf(err, _("'%s' exists; cannot create '%s'"), + dirname.buf, refname); + goto cleanup; + } + + if (extras && string_list_has_string(extras, dirname.buf)) { + strbuf_addf(err, _("cannot process '%s' and '%s' at the same time"), + refname, dirname.buf); + goto cleanup; + } + } /* - * We are still at a leading dir of the refname (e.g., - * "refs/foo"; if there is a reference with that name, - * it is a conflict, *unless* it is in skip. + * We are at the leaf of our refname (e.g., "refs/foo/bar"). + * There is no point in searching for a reference with that + * name, because a refname isn't considered to conflict with + * itself. But we still need to check for references whose + * names are in the "refs/foo/bar/" namespace, because they + * *do* conflict. */ - if (skip && string_list_has_string(skip, dirname.buf)) - continue; + strbuf_addstr(&dirname, refname + dirname.len); + strbuf_addch(&dirname, '/'); + + if (!initial_transaction) { + struct ref_iterator *iter; + int ok; + + iter = refs_ref_iterator_begin(refs, dirname.buf, NULL, 0, + DO_FOR_EACH_INCLUDE_BROKEN); + while ((ok = ref_iterator_advance(iter)) == ITER_OK) { + if (skip && + string_list_has_string(skip, iter->refname)) + continue; + + strbuf_addf(err, _("'%s' exists; cannot create '%s'"), + iter->refname, refname); + ref_iterator_abort(iter); + goto cleanup; + } - if (!initial_transaction && - !refs_read_raw_ref(refs, dirname.buf, &oid, &referent, - &type, &ignore_errno)) { - strbuf_addf(err, _("'%s' exists; cannot create '%s'"), - dirname.buf, refname); - goto cleanup; + if (ok != ITER_DONE) + BUG("error while iterating over references"); } - if (extras && string_list_has_string(extras, dirname.buf)) { + extra_refname = find_descendant_ref(dirname.buf, extras, skip); + if (extra_refname) { strbuf_addf(err, _("cannot process '%s' and '%s' at the same time"), - refname, dirname.buf); + refname, extra_refname); goto cleanup; } } - /* - * We are at the leaf of our refname (e.g., "refs/foo/bar"). - * There is no point in searching for a reference with that - * name, because a refname isn't considered to conflict with - * itself. But we still need to check for references whose - * names are in the "refs/foo/bar/" namespace, because they - * *do* conflict. - */ - strbuf_addstr(&dirname, refname + dirname.len); - strbuf_addch(&dirname, '/'); - - if (!initial_transaction) { - struct ref_iterator *iter; - int ok; - - iter = refs_ref_iterator_begin(refs, dirname.buf, NULL, 0, - DO_FOR_EACH_INCLUDE_BROKEN); - while ((ok = ref_iterator_advance(iter)) == ITER_OK) { - if (skip && - string_list_has_string(skip, iter->refname)) - continue; - - strbuf_addf(err, _("'%s' exists; cannot create '%s'"), - iter->refname, refname); - ref_iterator_abort(iter); - goto cleanup; - } - - if (ok != ITER_DONE) - BUG("error while iterating over references"); - } - - extra_refname = find_descendant_ref(dirname.buf, extras, skip); - if (extra_refname) - strbuf_addf(err, _("cannot process '%s' and '%s' at the same time"), - refname, extra_refname); - else - ret = 0; + ret = 0; cleanup: strbuf_release(&referent); @@ -2569,6 +2578,23 @@ int refs_verify_refname_available(struct ref_store *refs, return ret; } +int refs_verify_refname_available(struct ref_store *refs, + const char *refname, + const struct string_list *extras, + const struct string_list *skip, + unsigned int initial_transaction, + struct strbuf *err) +{ + struct string_list_item item = { .string = (char *) refname }; + struct string_list refnames = { + .items = &item, + .nr = 1, + }; + + return refs_verify_refnames_available(refs, &refnames, extras, skip, + initial_transaction, err); +} + struct do_for_each_reflog_help { each_reflog_fn *fn; void *cb_data; diff --git a/refs.h b/refs.h index a0cdd99250e..185aed5a461 100644 --- a/refs.h +++ b/refs.h @@ -124,6 +124,18 @@ int refs_verify_refname_available(struct ref_store *refs, unsigned int initial_transaction, struct strbuf *err); +/* + * Same as `refs_verify_refname_available()`, but checking for a list of + * refnames instead of only a single item. This is more efficient in the case + * where one needs to check multiple refnames. + */ +int refs_verify_refnames_available(struct ref_store *refs, + const struct string_list *refnames, + const struct string_list *extras, + const struct string_list *skip, + unsigned int initial_transaction, + struct strbuf *err); + int refs_ref_exists(struct ref_store *refs, const char *refname); int should_autocreate_reflog(enum log_refs_config log_all_ref_updates, From patchwork Wed Mar 12 15:56:11 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 14013660 Received: from fout-b4-smtp.messagingengine.com (fout-b4-smtp.messagingengine.com [202.12.124.147]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 73CFB25742B for ; Wed, 12 Mar 2025 15:56:20 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=202.12.124.147 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741794982; cv=none; b=AbACkQviacxlth1oCwVsBGIg3Q+asMsfILt3gJ8x3gRWaKGdGt8I98iiTnuXaG3qUfAengGp+RTMZg0G/1h04DaEd7nA8e6GlQcPI2WFVPKlEv0uZrkP99ndKZ+IB03eief0jSy705yH3O+I6V4gABdQzo0m4/s92bK7ZJndPRg= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741794982; c=relaxed/simple; bh=EjVHC1EIZ0R1PUcZLXpQz7r2BeoYcjhXsfoHHeAjovc=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:References: In-Reply-To:To:Cc; b=NAL2vLWBdPzjbFa7k1YX5VX4U4rZa4StYUpWc1sI8dfCHAkNT/OY+h0Wi3MNkeX6zv/YLAfMEg+VUIwqGeX9o6yceVEjlctuecyrS53gKHWiaLdqHSqFyjs4+IjJEGgNQLNr2EySge4HeFDu4VhSHkuIPJmupCdNkbBEWv/DGwc= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im; spf=pass smtp.mailfrom=pks.im; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b=VME3n04Y; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=ieKivxEq; arc=none smtp.client-ip=202.12.124.147 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pks.im Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="VME3n04Y"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="ieKivxEq" Received: from phl-compute-07.internal (phl-compute-07.phl.internal [10.202.2.47]) by mailfout.stl.internal (Postfix) with ESMTP id 89E5A11401FD; Wed, 12 Mar 2025 11:56:19 -0400 (EDT) Received: from phl-mailfrontend-01 ([10.202.2.162]) by phl-compute-07.internal (MEProxy); Wed, 12 Mar 2025 11:56:19 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :content-transfer-encoding:content-type:content-type:date:date :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to; s=fm1; t=1741794979; x=1741881379; bh=NYAu38YE8k8/ou1s8l7G5tJ/gDFRNhbdH1V2zh43yss=; b= VME3n04YB6YZFtPVB2o4Xpxr4TyYau6xxnc7SK0xhsUtYuUBpp4G+RAvYd2PiEDM 5MNDiAj5xo/hxHCK8ARzWoiI8RhLjH2PHVEdFE7gbCRzgSwuZ7l6nsnnHwvntYcJ QPb1nRIiKThPvDMFgBtnCzQwqBeNZYCDFS2DQsSgU3SLsjZsqyq6ltDJ/3KTY7rI XtN+hQMRDPak6A6a10a80SprGIun3/8io+M69nf3PUVaM3rNSrS9GOnnttTt+vJp K15IEJyyNR8Sr+bykdZI1I33slzZayWna0FmmHGHI/nX3txvIF40TbKk4sJNOiF5 bRJkLA/s+aP+K30YGj/wlg== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:content-type:date:date:feedback-id:feedback-id :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to:x-me-proxy :x-me-sender:x-me-sender:x-sasl-enc; s=fm1; t=1741794979; x= 1741881379; bh=NYAu38YE8k8/ou1s8l7G5tJ/gDFRNhbdH1V2zh43yss=; b=i eKivxEqRu+wSX892tsYz7T9lzSmOV4T6TcQ6YRkvPlG3oaTp3o175w54KZaDfvUu QkVS1rqUeScglaDX/68i7GFP8MLXgwJKE+wAjTi2Hf6VyPgtL8dne6SDH74gCZBv k0xYzT099T744iuF0VORUYrJfozCYExVgtMbrWSc3UqCkpORUbru4rtttIlepE1U jWmhtkJ49PDJ+zo7uaXlPrhS5VgZf3QbeRtx6AL5Qtsbws/+Sur+ZmS97qMPwaK2 qeejfE5NYvoGzoUA/+qvJ/sZdzAkJDKMG5ZeH5nP5eF1r3cH3Nr6Jr1zZ+jKCxHR Vor8BNX69th7kS25PjKkA== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeefvddrtddtgdduvdehgeekucetufdoteggodetrf dotffvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdggtfgfnhhsuhgsshgtrhhisggv pdfurfetoffkrfgpnffqhgenuceurghilhhouhhtmecufedttdenucesvcftvggtihhpih gvnhhtshculddquddttddmnecujfgurhephfffufggtgfgkfhfjgfvvefosehtjeertder tdejnecuhfhrohhmpefrrghtrhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhksh drihhmqeenucggtffrrghtthgvrhhnpeffueeiudejvdekheeuvdekfeffiedvueelteek udehjeetkeegvddugfdtgfeileenucevlhhushhtvghrufhiiigvpedunecurfgrrhgrmh epmhgrihhlfhhrohhmpehpshesphhkshdrihhmpdhnsggprhgtphhtthhopeejpdhmohgu vgepshhmthhpohhuthdprhgtphhtthhopehgihhtsehvghgvrhdrkhgvrhhnvghlrdhorh hgpdhrtghpthhtoheptghhrhhishgtohholhesthhugihfrghmihhlhidrohhrghdprhgt phhtthhopehpvghffhesphgvfhhfrdhnvghtpdhrtghpthhtohepshgrnhgurghlshestg hruhhsthihthhoohhthhhprghsthgvrdhnvghtpdhrtghpthhtohepshhhvghjihgrlhhu ohesghhmrghilhdrtghomhdprhgtphhtthhopehgihhtshhtvghrsehpohgsohigrdgtoh hmpdhrtghpthhtohepkhgrrhhthhhikhdrudekkeesghhmrghilhdrtghomh X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Wed, 12 Mar 2025 11:56:17 -0400 (EDT) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 922642fc (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Wed, 12 Mar 2025 15:56:14 +0000 (UTC) From: Patrick Steinhardt Date: Wed, 12 Mar 2025 16:56:11 +0100 Subject: [PATCH v6 05/16] refs/reftable: batch refname availability checks Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Message-Id: <20250312-pks-update-ref-optimization-v6-5-f778e0414f55@pks.im> References: <20250312-pks-update-ref-optimization-v6-0-f778e0414f55@pks.im> In-Reply-To: <20250312-pks-update-ref-optimization-v6-0-f778e0414f55@pks.im> To: git@vger.kernel.org Cc: Karthik Nayak , "brian m. carlson" , Jeff King , Junio C Hamano , shejialuo , Christian Couder X-Mailer: b4 0.14.2 Refactor the "reftable" backend to batch the availability check for refnames. This does not yet have an effect on performance as `refs_verify_refnames_available()` effectively still performs the availability check for each refname individually. But this will be optimized in subsequent commits, where we learn to optimize some parts of the logic when checking multiple refnames for availability. Signed-off-by: Patrick Steinhardt --- refs/reftable-backend.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c index d39a14c5a46..5c464b9d143 100644 --- a/refs/reftable-backend.c +++ b/refs/reftable-backend.c @@ -1069,6 +1069,7 @@ static int reftable_be_transaction_prepare(struct ref_store *ref_store, reftable_be_downcast(ref_store, REF_STORE_WRITE|REF_STORE_MAIN, "ref_transaction_prepare"); struct strbuf referent = STRBUF_INIT, head_referent = STRBUF_INIT; struct string_list affected_refnames = STRING_LIST_INIT_NODUP; + struct string_list refnames_to_check = STRING_LIST_INIT_NODUP; struct reftable_transaction_data *tx_data = NULL; struct reftable_backend *be; struct object_id head_oid; @@ -1224,12 +1225,7 @@ static int reftable_be_transaction_prepare(struct ref_store *ref_store, * can output a proper error message instead of failing * at a later point. */ - ret = refs_verify_refname_available(ref_store, u->refname, - &affected_refnames, NULL, - transaction->flags & REF_TRANSACTION_FLAG_INITIAL, - err); - if (ret < 0) - goto done; + string_list_append(&refnames_to_check, u->refname); /* * There is no need to write the reference deletion @@ -1379,6 +1375,12 @@ static int reftable_be_transaction_prepare(struct ref_store *ref_store, } } + ret = refs_verify_refnames_available(ref_store, &refnames_to_check, &affected_refnames, NULL, + transaction->flags & REF_TRANSACTION_FLAG_INITIAL, + err); + if (ret < 0) + goto done; + transaction->backend_data = tx_data; transaction->state = REF_TRANSACTION_PREPARED; @@ -1394,6 +1396,7 @@ static int reftable_be_transaction_prepare(struct ref_store *ref_store, string_list_clear(&affected_refnames, 0); strbuf_release(&referent); strbuf_release(&head_referent); + string_list_clear(&refnames_to_check, 0); return ret; } From patchwork Wed Mar 12 15:56:12 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 14013661 Received: from fout-b4-smtp.messagingengine.com (fout-b4-smtp.messagingengine.com [202.12.124.147]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 91F1625742D for ; Wed, 12 Mar 2025 15:56:20 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=202.12.124.147 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741794982; cv=none; b=Qa39rcTIDRPTKW/bOeRMo7KSmF/2sb+P6Q/W6tJqgTWrtriEiCwcwhUqeGh6sQqig2aVM+lYNMAHYouEf7UKrwMmhN/+NZHozyEUPhpdGez3LA4CNe8rSmdDF9zXopww6lu4t9vwoLDR23WnqmCNrDu12kF+Uz1iNv02w2Hfpjs= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741794982; c=relaxed/simple; bh=y3BNcvNA4weevNUmMKoi5Tt8ENuTelATf7Azcf1r1vg=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:References: In-Reply-To:To:Cc; b=GiBVeCOchCuwpd4m5eYuSe+fIcqMDFeNHhyIW7X2yhSBa5nij+btpFimZfSiUMIoFYYQ0R+wSpekiLCPYDiTTwSrYyV6We89xJ7Rzp91zJciBhjzj8bckHUZGtIozZ4AUlT+ddLU+nLoFHqOA5vj97RuziJ1+0LUvY3AmZ71dQk= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im; spf=pass smtp.mailfrom=pks.im; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b=HvRb7ry3; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=bbwD1tcA; arc=none smtp.client-ip=202.12.124.147 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pks.im Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="HvRb7ry3"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="bbwD1tcA" Received: from phl-compute-07.internal (phl-compute-07.phl.internal [10.202.2.47]) by mailfout.stl.internal (Postfix) with ESMTP id 82F7511401C7; Wed, 12 Mar 2025 11:56:19 -0400 (EDT) Received: from phl-mailfrontend-02 ([10.202.2.163]) by phl-compute-07.internal (MEProxy); Wed, 12 Mar 2025 11:56:19 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :content-transfer-encoding:content-type:content-type:date:date :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to; s=fm1; t=1741794979; x=1741881379; bh=2o8gE5g7K0oRxDtOXoeGg/dRAswONe72abRGXViA7q0=; b= HvRb7ry3yQkgmSTIjVYy2m57/ww1+Z0fXtCKjqX2adbNv2HUwnhgGpFVah9qrs7S vidRaAsTF71e1UAquArVnmgivCp5w3pD3HAsXhiWhmIcl1ZNuQ3QaI9qDO7RSpyp anPXSG79XrFKs1C35AhCGj5+0B7CxlZyamiq8p7kFKV4ZeHagRhe1r5/b+JuQ4Vu 6Jt8yM9FYovOm5cTgJxrc3SKlHB3OY4LOWQRe5ui+98un9fBZa/MbYS8gFonk8Tt /GjiHJJBgVTjf8p5/URyREc8aXl4Dqt/yG+LcU86ohc5bbuHP14KA7hUCV54t8/5 mUzvMLS21L21/3oeyau9iQ== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:content-type:date:date:feedback-id:feedback-id :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to:x-me-proxy :x-me-sender:x-me-sender:x-sasl-enc; s=fm1; t=1741794979; x= 1741881379; bh=2o8gE5g7K0oRxDtOXoeGg/dRAswONe72abRGXViA7q0=; b=b bwD1tcAJnmYRakCYCbmKl1hC21uelnDnsxpkhGxNFqbYA6dd/43xHwq5HsTYcACt glRQXVYHi4D2gzV4nmSF4pS0yWqxHtnU92WLQBJiO/OmklLBmfl6RXdCQxvw2/Ja ftyTswPeckz5Casumyh1Y1aZdOHZmAJwfxsnDPNsM4oEwVM7lOeKGOlxZAr/gO8l Kn+ssCd2H/WShx2oqJSWEOn+7yDOX5XHtDyzSHOqt+Y1GUytGAs1lsxIkkTuPj+S QdBOWPFtVfRZcjDxIQgHC3CI2nfQa/y367eSjkQ+4IAARzQ6bIoKtGgMhUTEHDH/ RI7TuQsqbXao/s88wyOkQ== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeefvddrtddtgdduvdehgeekucetufdoteggodetrf dotffvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdggtfgfnhhsuhgsshgtrhhisggv pdfurfetoffkrfgpnffqhgenuceurghilhhouhhtmecufedttdenucesvcftvggtihhpih gvnhhtshculddquddttddmnecujfgurhephfffufggtgfgkfhfjgfvvefosehtjeertder tdejnecuhfhrohhmpefrrghtrhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhksh drihhmqeenucggtffrrghtthgvrhhnpeffueeiudejvdekheeuvdekfeffiedvueelteek udehjeetkeegvddugfdtgfeileenucevlhhushhtvghrufhiiigvpedunecurfgrrhgrmh epmhgrihhlfhhrohhmpehpshesphhkshdrihhmpdhnsggprhgtphhtthhopeejpdhmohgu vgepshhmthhpohhuthdprhgtphhtthhopegthhhrihhstghoohhlsehtuhigfhgrmhhilh ihrdhorhhgpdhrtghpthhtohepshhhvghjihgrlhhuohesghhmrghilhdrtghomhdprhgt phhtthhopehgihhtshhtvghrsehpohgsohigrdgtohhmpdhrtghpthhtohepshgrnhgurg hlshestghruhhsthihthhoohhthhhprghsthgvrdhnvghtpdhrtghpthhtohepphgvfhhf sehpvghffhdrnhgvthdprhgtphhtthhopehgihhtsehvghgvrhdrkhgvrhhnvghlrdhorh hgpdhrtghpthhtohepkhgrrhhthhhikhdrudekkeesghhmrghilhdrtghomh X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Wed, 12 Mar 2025 11:56:17 -0400 (EDT) Received: by vm-mail (OpenSMTPD) with ESMTPSA id aa4cfac7 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Wed, 12 Mar 2025 15:56:15 +0000 (UTC) From: Patrick Steinhardt Date: Wed, 12 Mar 2025 16:56:12 +0100 Subject: [PATCH v6 06/16] refs/files: batch refname availability checks for normal transactions Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Message-Id: <20250312-pks-update-ref-optimization-v6-6-f778e0414f55@pks.im> References: <20250312-pks-update-ref-optimization-v6-0-f778e0414f55@pks.im> In-Reply-To: <20250312-pks-update-ref-optimization-v6-0-f778e0414f55@pks.im> To: git@vger.kernel.org Cc: Karthik Nayak , "brian m. carlson" , Jeff King , Junio C Hamano , shejialuo , Christian Couder X-Mailer: b4 0.14.2 Same as the "reftable" backend that we have adapted in the preceding commit to use batched refname availability checks we can also do so for the "files" backend. Things are a bit more intricate here though, as we call `refs_verify_refname_available()` in a set of different contexts: 1. `lock_raw_ref()` when it hits either EEXISTS or EISDIR when creating a new reference, mostly to create a nice, user-readable error message. This is nothing we have to care about too much, as we only hit this code path at most once when we hit a conflict. 2. `lock_raw_ref()` when it _could_ create the lockfile to check whether it is conflicting with any packed refs. In the general case, this code path will be hit once for every (successful) reference update. 3. `lock_ref_oid_basic()`, but it is only executed when copying or renaming references or when expiring reflogs. It will thus not be called in contexts where we have many references queued up. 4. `refs_refname_ref_available()`, but again only when copying or renaming references. It is thus not interesting due to the same reason as the previous case. 5. `files_transaction_finish_initial()`, which is only executed when creating a new repository or migrating references. So out of these, only (2) and (5) are viable candidates to use the batched checks. Adapt `lock_raw_ref()` accordingly by queueing up reference names that need to be checked for availability and then checking them after we have processed all updates. This check is done before we (optionally) lock the `packed-refs` file, which is somewhat flawed because it means that the `packed-refs` could still change after the availability check and thus create an undetected conflict. But unconditionally locking the file would change semantics that users are likely to rely on, so we keep the current locking sequence intact, even if it's suboptmial. The refactoring of `files_transaction_finish_initial()` will be done in the next commit. Signed-off-by: Patrick Steinhardt --- refs/files-backend.c | 42 +++++++++++++++++++++++++++++++----------- 1 file changed, 31 insertions(+), 11 deletions(-) diff --git a/refs/files-backend.c b/refs/files-backend.c index 29f08dced40..f798d8dae37 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -678,6 +678,7 @@ static void unlock_ref(struct ref_lock *lock) */ static int lock_raw_ref(struct files_ref_store *refs, const char *refname, int mustexist, + struct string_list *refnames_to_check, const struct string_list *extras, struct ref_lock **lock_p, struct strbuf *referent, @@ -855,16 +856,11 @@ static int lock_raw_ref(struct files_ref_store *refs, } /* - * If the ref did not exist and we are creating it, - * make sure there is no existing packed ref that - * conflicts with refname: + * If the ref did not exist and we are creating it, we have to + * make sure there is no existing packed ref that conflicts + * with refname. This check is deferred so that we can batch it. */ - if (refs_verify_refname_available( - refs->packed_ref_store, refname, - extras, NULL, 0, err)) { - ret = TRANSACTION_NAME_CONFLICT; - goto error_return; - } + string_list_append(refnames_to_check, refname); } ret = 0; @@ -2569,6 +2565,7 @@ static int lock_ref_for_update(struct files_ref_store *refs, struct ref_update *update, struct ref_transaction *transaction, const char *head_ref, + struct string_list *refnames_to_check, struct string_list *affected_refnames, struct strbuf *err) { @@ -2597,7 +2594,7 @@ static int lock_ref_for_update(struct files_ref_store *refs, lock->count++; } else { ret = lock_raw_ref(refs, update->refname, mustexist, - affected_refnames, + refnames_to_check, affected_refnames, &lock, &referent, &update->type, err); if (ret) { @@ -2811,6 +2808,7 @@ static int files_transaction_prepare(struct ref_store *ref_store, size_t i; int ret = 0; struct string_list affected_refnames = STRING_LIST_INIT_NODUP; + struct string_list refnames_to_check = STRING_LIST_INIT_NODUP; char *head_ref = NULL; int head_type; struct files_transaction_backend_data *backend_data; @@ -2898,7 +2896,8 @@ static int files_transaction_prepare(struct ref_store *ref_store, struct ref_update *update = transaction->updates[i]; ret = lock_ref_for_update(refs, update, transaction, - head_ref, &affected_refnames, err); + head_ref, &refnames_to_check, + &affected_refnames, err); if (ret) goto cleanup; @@ -2930,6 +2929,26 @@ static int files_transaction_prepare(struct ref_store *ref_store, } } + /* + * Verify that none of the loose reference that we're about to write + * conflict with any existing packed references. Ideally, we'd do this + * check after the packed-refs are locked so that the file cannot + * change underneath our feet. But introducing such a lock now would + * probably do more harm than good as users rely on there not being a + * global lock with the "files" backend. + * + * Another alternative would be to do the check after the (optional) + * lock, but that would extend the time we spend in the globally-locked + * state. + * + * So instead, we accept the race for now. + */ + if (refs_verify_refnames_available(refs->packed_ref_store, &refnames_to_check, + &affected_refnames, NULL, 0, err)) { + ret = TRANSACTION_NAME_CONFLICT; + goto cleanup; + } + if (packed_transaction) { if (packed_refs_lock(refs->packed_ref_store, 0, err)) { ret = TRANSACTION_GENERIC_ERROR; @@ -2972,6 +2991,7 @@ static int files_transaction_prepare(struct ref_store *ref_store, cleanup: free(head_ref); string_list_clear(&affected_refnames, 0); + string_list_clear(&refnames_to_check, 0); if (ret) files_transaction_cleanup(refs, transaction); From patchwork Wed Mar 12 15:56:13 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 14013662 Received: from fhigh-b4-smtp.messagingengine.com (fhigh-b4-smtp.messagingengine.com [202.12.124.155]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 4C3D92571A6 for ; Wed, 12 Mar 2025 15:56:22 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=202.12.124.155 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741794983; cv=none; b=FG2l055Di8dpW48M3cuWb/K4r8kvbolgs9+iUW3HypQKQGMYE5qW1nmd2iEMkHWgfcciOtLD4zjWyt+wlmD6LxPJOqxtgmPkOV0e487zhYs42jPye4Y7+xRZWwJKaf1JmHXrAWrWhMW8tBv+5hPcgu/NnKOxGn+Ts8yobmOiWIw= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741794983; c=relaxed/simple; bh=ydhf6faLjUx358ZKxeoqdO6qcM0iDhSqsLPHqNc8N9Y=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:References: In-Reply-To:To:Cc; b=pJVUGe2BsPrj0MjPeHdhyRQPshikJEcxqBhMX2WrZ2BGsMv/F865UCm7CLCI4VFSLjaAgHlpnG9DLWIEHTFhhPdF7AF1DW7+T6BTRWneJEA95H32iIxLvZ6Q3eYIJN50gVrYmHEZK9jqJPOMOIPNlYofWVKj1w0FTnA411ViK/Q= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im; spf=pass smtp.mailfrom=pks.im; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b=Hxh0wD3c; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=m4/ycQVo; arc=none smtp.client-ip=202.12.124.155 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pks.im Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="Hxh0wD3c"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="m4/ycQVo" Received: from phl-compute-07.internal (phl-compute-07.phl.internal [10.202.2.47]) by mailfhigh.stl.internal (Postfix) with ESMTP id 6DA442540278; Wed, 12 Mar 2025 11:56:21 -0400 (EDT) Received: from phl-mailfrontend-02 ([10.202.2.163]) by phl-compute-07.internal (MEProxy); Wed, 12 Mar 2025 11:56:21 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :content-transfer-encoding:content-type:content-type:date:date :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to; s=fm1; t=1741794981; x=1741881381; bh=dDLePwdbVU1ZlvsNM94bf6NVDVXMvKfsV5EV+vtUfUs=; b= Hxh0wD3cgBM+EUhqi1JbWls/oZUYgRCvJHtmIuOvbYPorvWrFQ97LhSVOXdttytx CeFfaKYzyuPzoVs/ojvjJQGfizj2rphMlNpS4lw1b7ycyeGj1wfNKkDUdwRJ0kP/ npO3qJYL7FstFqVK3JxwQjwrSyK3Uk6mNb+y+M59j5k6aGQWyObMgnBsxy2l9NqN NzldL/5+917gsMwsq/khiOjAJGMUXXMXW0Ct+zxbBm5+1vQnJCNTtx7jf6jvphFa ASI+rV6CNJ/M5xJJbUBboKtlceXUiVYg1mjwl57B4LPJ1qiZMdKtE1Pg1c/WPdWb Kkxde7Lbq5Dc5A+pbE39Zw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:content-type:date:date:feedback-id:feedback-id :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to:x-me-proxy :x-me-sender:x-me-sender:x-sasl-enc; s=fm1; t=1741794981; x= 1741881381; bh=dDLePwdbVU1ZlvsNM94bf6NVDVXMvKfsV5EV+vtUfUs=; b=m 4/ycQVosV2IdXiVkQ0Rv7MvcDcAoQtFkg3kolqkdborbbr27b0dNmXNXgWm+yotO Gf4DDnaoq64NjqxJDzea0oLamroRjzfgsgV6k1z5Eug758c+UYri7WSxnHw1RVi4 mxUIYPak7D2iRA8JWWGXrW1kZhy6CnDC7nt7VwzR1DBwk0mX5slk55kODr7VbIcv KktRLUWhweaKcl9GWbm+JZWs1in1JBUlS21ceDU6/blpZh8VDsFfllK67e/qfkHk xkYL/+uHG/40cXH+Suc6HhVVVpiglr9NjurdKnDl525sIGx/V+c2sGogMqx+9DzA Xb/NxW7OAqlzMa5HNeamg== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeefvddrtddtgdduvdehgeekucetufdoteggodetrf dotffvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdggtfgfnhhsuhgsshgtrhhisggv pdfurfetoffkrfgpnffqhgenuceurghilhhouhhtmecufedttdenucesvcftvggtihhpih gvnhhtshculddquddttddmnecujfgurhephfffufggtgfgkfhfjgfvvefosehtjeertder tdejnecuhfhrohhmpefrrghtrhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhksh drihhmqeenucggtffrrghtthgvrhhnpeffueeiudejvdekheeuvdekfeffiedvueelteek udehjeetkeegvddugfdtgfeileenucevlhhushhtvghrufhiiigvpedunecurfgrrhgrmh epmhgrihhlfhhrohhmpehpshesphhkshdrihhmpdhnsggprhgtphhtthhopeejpdhmohgu vgepshhmthhpohhuthdprhgtphhtthhopehgihhtsehvghgvrhdrkhgvrhhnvghlrdhorh hgpdhrtghpthhtoheptghhrhhishgtohholhesthhugihfrghmihhlhidrohhrghdprhgt phhtthhopehpvghffhesphgvfhhfrdhnvghtpdhrtghpthhtohepshhhvghjihgrlhhuoh esghhmrghilhdrtghomhdprhgtphhtthhopehkrghrthhhihhkrddukeeksehgmhgrihhl rdgtohhmpdhrtghpthhtohepghhithhsthgvrhesphhosghogidrtghomhdprhgtphhtth hopehsrghnuggrlhhssegtrhhushhthihtohhothhhphgrshhtvgdrnhgvth X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Wed, 12 Mar 2025 11:56:19 -0400 (EDT) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 38399114 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Wed, 12 Mar 2025 15:56:16 +0000 (UTC) From: Patrick Steinhardt Date: Wed, 12 Mar 2025 16:56:13 +0100 Subject: [PATCH v6 07/16] refs/files: batch refname availability checks for initial transactions Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Message-Id: <20250312-pks-update-ref-optimization-v6-7-f778e0414f55@pks.im> References: <20250312-pks-update-ref-optimization-v6-0-f778e0414f55@pks.im> In-Reply-To: <20250312-pks-update-ref-optimization-v6-0-f778e0414f55@pks.im> To: git@vger.kernel.org Cc: Karthik Nayak , "brian m. carlson" , Jeff King , Junio C Hamano , shejialuo , Christian Couder X-Mailer: b4 0.14.2 The "files" backend explicitly carves out special logic for its initial transaction so that it can avoid writing out every single reference as a loose reference. While the assumption is that there shouldn't be any preexisting references, we still have to verify that none of the newly written references will conflict with any other new reference in the same transaction. Refactor the initial transaction to use batched refname availability checks. This does not yet have an effect on performance as we still call `refs_verify_refname_available()` in a loop. But this will change in subsequent commits and then impact performance when cloning a repository with many references or when migrating references to the "files" format. This will improve performance when cloning a repository with many references or when migrating references from any format to the "files" format once the availability checks have learned to optimize checks for many references in a subsequent commit. Signed-off-by: Patrick Steinhardt --- refs/files-backend.c | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/refs/files-backend.c b/refs/files-backend.c index f798d8dae37..ab6f0af5502 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -3056,6 +3056,7 @@ static int files_transaction_finish_initial(struct files_ref_store *refs, size_t i; int ret = 0; struct string_list affected_refnames = STRING_LIST_INIT_NODUP; + struct string_list refnames_to_check = STRING_LIST_INIT_NODUP; struct ref_transaction *packed_transaction = NULL; struct ref_transaction *loose_transaction = NULL; @@ -3105,11 +3106,7 @@ static int files_transaction_finish_initial(struct files_ref_store *refs, !is_null_oid(&update->old_oid)) BUG("initial ref transaction with old_sha1 set"); - if (refs_verify_refname_available(&refs->base, update->refname, - &affected_refnames, NULL, 1, err)) { - ret = TRANSACTION_NAME_CONFLICT; - goto cleanup; - } + string_list_append(&refnames_to_check, update->refname); /* * packed-refs don't support symbolic refs, root refs and reflogs, @@ -3145,8 +3142,19 @@ static int files_transaction_finish_initial(struct files_ref_store *refs, } } - if (packed_refs_lock(refs->packed_ref_store, 0, err) || - ref_transaction_commit(packed_transaction, err)) { + if (packed_refs_lock(refs->packed_ref_store, 0, err)) { + ret = TRANSACTION_GENERIC_ERROR; + goto cleanup; + } + + if (refs_verify_refnames_available(&refs->base, &refnames_to_check, + &affected_refnames, NULL, 1, err)) { + packed_refs_unlock(refs->packed_ref_store); + ret = TRANSACTION_NAME_CONFLICT; + goto cleanup; + } + + if (ref_transaction_commit(packed_transaction, err)) { ret = TRANSACTION_GENERIC_ERROR; goto cleanup; } @@ -3167,6 +3175,7 @@ static int files_transaction_finish_initial(struct files_ref_store *refs, ref_transaction_free(packed_transaction); transaction->state = REF_TRANSACTION_CLOSED; string_list_clear(&affected_refnames, 0); + string_list_clear(&refnames_to_check, 0); return ret; } From patchwork Wed Mar 12 15:56:14 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 14013663 Received: from fout-b4-smtp.messagingengine.com (fout-b4-smtp.messagingengine.com [202.12.124.147]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 9DEAE257AFE for ; Wed, 12 Mar 2025 15:56:22 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=202.12.124.147 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741794984; cv=none; b=dlk+f7krXB05MUOVA4OUl3I050qyDqiE34BXZslrcjvW4R5YoMhsjUswhxgWplm3+K9lcvkKvS9LUSXvdiM0MnBESeWS0jGv/jMki1n9suse6JsA9FRHjvwxIl0DsXehzkLGgrlgQ2hja7DPt3aTkQuAxzo0I5R1wTwzDRSLXPA= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741794984; c=relaxed/simple; bh=iLaJ3iVPCyvhpkTp+4LGCrn9MEugnEuY2pAYxiG3+7Y=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:References: In-Reply-To:To:Cc; b=Q8vFNDs5fWKrtCwjqcX1GZC0XRvSHHcTQxuIaTYDzZyk9rkXbEEhv0AooE53FQBSNZf4rWIYlmpR6lwblq2EpAaDnwqLYy7InRETiWmYjjaLe66EZfL8SlMc94m/SEA6ErOxXMiOJyaYYgM7WHY0lMDLUryri1TFrksq0+W0eWM= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im; spf=pass smtp.mailfrom=pks.im; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b=gHbXT5Dj; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=q4J4HkLW; arc=none smtp.client-ip=202.12.124.147 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pks.im Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="gHbXT5Dj"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="q4J4HkLW" Received: from phl-compute-07.internal (phl-compute-07.phl.internal [10.202.2.47]) by mailfout.stl.internal (Postfix) with ESMTP id 8C9F91140226; Wed, 12 Mar 2025 11:56:21 -0400 (EDT) Received: from phl-mailfrontend-01 ([10.202.2.162]) by phl-compute-07.internal (MEProxy); Wed, 12 Mar 2025 11:56:21 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :content-transfer-encoding:content-type:content-type:date:date :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to; s=fm1; t=1741794981; x=1741881381; bh=gvhbxVPaOmGQyzRN5mSwiB2NzvhSzaeeZf0y+ELUFKY=; b= gHbXT5DjbQIhaVNpGLj3PWzQ3Dw6yVIPYIt7gMmYViW8CmIRBaEzELr4fli/J80Q DI51T/kFOrMRVBgU6o9G4IL4n81aPfgNclw+sderW7zqbgg0bOVnXRGThlYS27QZ //QD514w4kolcQeABZ/kQCMcu9YKRu++KOSAP1RowMCW9vLNiL/vmEaAh0qDtobd VDf4SVGcb2C4QM5IwHTPCZXwLx1S32utKgIHwEUZ6FluSzgfNMa17kIHB/YWu11l 01tlMwq4lEIo9AUvf+q64tkesQfA4vxoSk71Ca1sUrY4wZGUKHRwK0bRLOv8dYv0 N5PvNHe8EnH7EuQXJvRkJQ== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:content-type:date:date:feedback-id:feedback-id :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to:x-me-proxy :x-me-sender:x-me-sender:x-sasl-enc; s=fm1; t=1741794981; x= 1741881381; bh=gvhbxVPaOmGQyzRN5mSwiB2NzvhSzaeeZf0y+ELUFKY=; b=q 4J4HkLWkrSwD3NVjlLRZaV/MF6KrQ49ZTs2Uvv2BWph+8FDHTYmck2ervalPsABB 8HGnfNSSPHjPxgnlDDUKGVRxR6JXFcYFucfoHLLTBusqFCeAhcvRqxzFSeMH2pvk Y6AwT8sgBlrVRTu5pCkyHz+AHFCCiRPJ3cogzg7hmZHczGItUwSeTAoFb1nUPkkp oTDbIfKoBMfUFVu+a9DqTSDEJf7nARAEs3EoLf/k5js5ydfXYwhUN2JrjTGm3DAj RXTw7U0G1TxQg+QygYGD8QxiUQQ68tcPmQS3dT33W/XoHdLh44/MJgbdu7DEwawl c0I3knBDiER42jkcXq5tw== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeefvddrtddtgdduvdehgeekucetufdoteggodetrf dotffvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdggtfgfnhhsuhgsshgtrhhisggv pdfurfetoffkrfgpnffqhgenuceurghilhhouhhtmecufedttdenucesvcftvggtihhpih gvnhhtshculddquddttddmnecujfgurhephfffufggtgfgkfhfjgfvvefosehtkeertder tdejnecuhfhrohhmpefrrghtrhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhksh drihhmqeenucggtffrrghtthgvrhhnpeefhfeugeelheefjeektdffhedvhfdvteefgfdt udffudevveetgeeuuedtkefhgeenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmh epmhgrihhlfhhrohhmpehpshesphhkshdrihhmpdhnsggprhgtphhtthhopeejpdhmohgu vgepshhmthhpohhuthdprhgtphhtthhopegthhhrihhstghoohhlsehtuhigfhgrmhhilh ihrdhorhhgpdhrtghpthhtohepshgrnhgurghlshestghruhhsthihthhoohhthhhprghs thgvrdhnvghtpdhrtghpthhtohepphgvfhhfsehpvghffhdrnhgvthdprhgtphhtthhope hshhgvjhhirghluhhosehgmhgrihhlrdgtohhmpdhrtghpthhtohepghhithesvhhgvghr rdhkvghrnhgvlhdrohhrghdprhgtphhtthhopehgihhtshhtvghrsehpohgsohigrdgtoh hmpdhrtghpthhtohepkhgrrhhthhhikhdrudekkeesghhmrghilhdrtghomh X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Wed, 12 Mar 2025 11:56:19 -0400 (EDT) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 5837283a (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Wed, 12 Mar 2025 15:56:17 +0000 (UTC) From: Patrick Steinhardt Date: Wed, 12 Mar 2025 16:56:14 +0100 Subject: [PATCH v6 08/16] refs: stop re-verifying common prefixes for availability Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Message-Id: <20250312-pks-update-ref-optimization-v6-8-f778e0414f55@pks.im> References: <20250312-pks-update-ref-optimization-v6-0-f778e0414f55@pks.im> In-Reply-To: <20250312-pks-update-ref-optimization-v6-0-f778e0414f55@pks.im> To: git@vger.kernel.org Cc: Karthik Nayak , "brian m. carlson" , Jeff King , Junio C Hamano , shejialuo , Christian Couder X-Mailer: b4 0.14.2 One of the checks done by `refs_verify_refnames_available()` is whether any of the prefixes of a reference already exists. For example, given a reference "refs/heads/main", we'd check whether "refs/heads" or "refs" already exist, and if so we'd abort the transaction. When updating multiple references at once, this check is performed for each of the references individually. Consequently, because references tend to have common prefixes like "refs/heads/" or refs/tags/", we evaluate the availability of these prefixes repeatedly. Naturally this is a waste of compute, as the availability of those prefixes should in general not change in the middle of a transaction. And if it would, backends would notice at a later point in time. Optimize this pattern by storing prefixes in a `strset` so that we can trivially track those prefixes that we have already checked. This leads to a significant speedup with the "reftable" backend when creating many references that all share a common prefix: Benchmark 1: update-ref: create many refs (refformat = reftable, preexisting = 100000, new = 10000, revision = HEAD~) Time (mean ± σ): 63.1 ms ± 1.8 ms [User: 41.0 ms, System: 21.6 ms] Range (min … max): 60.6 ms … 69.5 ms 38 runs Benchmark 2: update-ref: create many refs (refformat = reftable, preexisting = 100000, new = 10000, revision = HEAD) Time (mean ± σ): 40.0 ms ± 1.3 ms [User: 29.3 ms, System: 10.3 ms] Range (min … max): 38.1 ms … 47.3 ms 61 runs Summary update-ref: create many refs (refformat = reftable, preexisting = 100000, new = 10000, revision = HEAD) ran 1.58 ± 0.07 times faster than update-ref: create many refs (refformat = reftable, preexisting = 100000, new = 10000, revision = HEAD~) For the "files" backend we see an improvement, but a much smaller one: Benchmark 1: update-ref: create many refs (refformat = files, preexisting = 100000, new = 10000, revision = HEAD~) Time (mean ± σ): 395.8 ms ± 5.3 ms [User: 63.6 ms, System: 330.5 ms] Range (min … max): 387.0 ms … 404.6 ms 10 runs Benchmark 2: update-ref: create many refs (refformat = files, preexisting = 100000, new = 10000, revision = HEAD) Time (mean ± σ): 386.0 ms ± 4.0 ms [User: 51.5 ms, System: 332.8 ms] Range (min … max): 380.8 ms … 392.6 ms 10 runs Summary update-ref: create many refs (refformat = files, preexisting = 100000, new = 10000, revision = HEAD) ran 1.03 ± 0.02 times faster than update-ref: create many refs (refformat = files, preexisting = 100000, new = 10000, revision = HEAD~) This change also leads to a modest improvement when writing references with "initial" semantics, for example when migrating references. The following benchmarks are migrating 1m references from the "reftable" to the "files" backend: Benchmark 1: migrate reftable:files (refcount = 1000000, revision = HEAD~) Time (mean ± σ): 836.6 ms ± 5.6 ms [User: 645.2 ms, System: 185.2 ms] Range (min … max): 829.6 ms … 845.9 ms 10 runs Benchmark 2: migrate reftable:files (refcount = 1000000, revision = HEAD) Time (mean ± σ): 759.8 ms ± 5.1 ms [User: 574.9 ms, System: 178.9 ms] Range (min … max): 753.1 ms … 768.8 ms 10 runs Summary migrate reftable:files (refcount = 1000000, revision = HEAD) ran 1.10 ± 0.01 times faster than migrate reftable:files (refcount = 1000000, revision = HEAD~) And vice versa: Benchmark 1: migrate files:reftable (refcount = 1000000, revision = HEAD~) Time (mean ± σ): 870.7 ms ± 5.7 ms [User: 735.2 ms, System: 127.4 ms] Range (min … max): 861.6 ms … 883.2 ms 10 runs Benchmark 2: migrate files:reftable (refcount = 1000000, revision = HEAD) Time (mean ± σ): 799.1 ms ± 8.5 ms [User: 661.1 ms, System: 130.2 ms] Range (min … max): 787.5 ms … 812.6 ms 10 runs Summary migrate files:reftable (refcount = 1000000, revision = HEAD) ran 1.09 ± 0.01 times faster than migrate files:reftable (refcount = 1000000, revision = HEAD~) The impact here is significantly smaller given that we don't perform any reference reads with "initial" semantics, so the speedup only comes from us doing less string list lookups. Signed-off-by: Patrick Steinhardt --- refs.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/refs.c b/refs.c index d91a2184e06..111e3cf3aa9 100644 --- a/refs.c +++ b/refs.c @@ -2477,6 +2477,7 @@ int refs_verify_refnames_available(struct ref_store *refs, struct strbuf dirname = STRBUF_INIT; struct strbuf referent = STRBUF_INIT; struct string_list_item *item; + struct strset dirnames; int ret = -1; /* @@ -2486,6 +2487,8 @@ int refs_verify_refnames_available(struct ref_store *refs, assert(err); + strset_init(&dirnames); + for_each_string_list_item(item, refnames) { const char *refname = item->string; const char *extra_refname; @@ -2515,6 +2518,14 @@ int refs_verify_refnames_available(struct ref_store *refs, if (skip && string_list_has_string(skip, dirname.buf)) continue; + /* + * If we've already seen the directory we don't need to + * process it again. Skip it to avoid checking checking + * common prefixes like "refs/heads/" repeatedly. + */ + if (!strset_add(&dirnames, dirname.buf)) + continue; + if (!initial_transaction && !refs_read_raw_ref(refs, dirname.buf, &oid, &referent, &type, &ignore_errno)) { @@ -2575,6 +2586,7 @@ int refs_verify_refnames_available(struct ref_store *refs, cleanup: strbuf_release(&referent); strbuf_release(&dirname); + strset_clear(&dirnames); return ret; } From patchwork Wed Mar 12 15:56:15 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 14013666 Received: from fout-b4-smtp.messagingengine.com (fout-b4-smtp.messagingengine.com [202.12.124.147]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id C00E3258CCD for ; Wed, 12 Mar 2025 15:56:24 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=202.12.124.147 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741794987; cv=none; b=rOrAsG/KWwsvvE/tYKTAOXKMsskJexm6GeF48uOV0D61lyjo1FCVlk7s+PX4Ozy4F+aYMlJXb6GQx1HZ1G2gOEoxbBhWdemwaGX2tfDHaQctHKDVktS6FU5LmqtjHsMGTbZagIRiqU2kHkSTD8sVvy/z0IJAdble84hMgR70OqY= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741794987; c=relaxed/simple; bh=3FplRfp2Ds5NRJAXwxYt1mBchNNfH14Qs11OHZBgHZ8=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:References: In-Reply-To:To:Cc; b=B6apxr7UAzI7wbQOxgEJPkxG4KMKJUyoD/QZn+kA6s8FotSvEIlsLbcl+1umVvNpeLa8OApGn3JwoSPRHHsEsa3eBHHX7NP5VnzhNlkZ27F0uJaTNZ7qGSBWEdHCOdDdMVJlaD5SkmOQsSlYVJ1m0Zkhvpz6TzQtp7bSJKuJY/Q= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im; spf=pass smtp.mailfrom=pks.im; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b=lqZw98rw; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=iqLrEaFp; arc=none smtp.client-ip=202.12.124.147 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pks.im Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="lqZw98rw"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="iqLrEaFp" Received: from phl-compute-08.internal (phl-compute-08.phl.internal [10.202.2.48]) by mailfout.stl.internal (Postfix) with ESMTP id 9DABC11401E3; Wed, 12 Mar 2025 11:56:23 -0400 (EDT) Received: from phl-mailfrontend-02 ([10.202.2.163]) by phl-compute-08.internal (MEProxy); Wed, 12 Mar 2025 11:56:23 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :content-transfer-encoding:content-type:content-type:date:date :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to; s=fm1; t=1741794983; x=1741881383; bh=K0GDQhy/+0vJFT3aXqI9k3+YIHZJQapayvsK3lR8OW0=; b= lqZw98rwWnVF6ZXgT3+rqO3WKIfGuFneyFW77ZiEQTVJMWqufxpeagsv7iFZeVp5 dDdDVqlfVpmKZvSAldl7ldtE8yxipyu7buF72KnQdWaOL3nxQRv2c3duNGxZpjtV ChSNqShQ3wUn0lbQSNh3wP9PnIjxlx6pDRib0r4sLMqeg3QaKufE8pmhXyeg0shk B2MqI+V7hE8mSSASbdL2aRY0erImpzlZUqMgHV8zYOUELmGPohtPvXjY5dFz/UEg PF9e67SLsVdfNjk6NY/BRJKz/mAuxCDuMpdFJ//YVQHp6+mVUhpmQiRmqhRqdJ+u G/UddmQCk0D2qfDptFTZdw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:content-type:date:date:feedback-id:feedback-id :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to:x-me-proxy :x-me-sender:x-me-sender:x-sasl-enc; s=fm1; t=1741794983; x= 1741881383; bh=K0GDQhy/+0vJFT3aXqI9k3+YIHZJQapayvsK3lR8OW0=; b=i qLrEaFphnawjefJtk/MkLQXMPTWXWrKu4ZcI+NqYfhxDMsG3J2i08lYWTIHiOu29 jKFmG3Li38Sh6i9pZ1M6jVV2lF+25783zKadcnLL11BI9IYFaNiuZRx78cQweSm1 DLAmvO2D04ayNxIYGWekKVP42gRq9+pfjUIFl7HrDo5P/KcAoex+ZEnUvGh9mutz qddEddmb2vfgUysvog9laewEtAK3IRRVJsjt5fPqvBAemQi3P0+vIQobLCa5zKR1 g2ePdCwBCwy0dBgW47bc0DohC2S3gjAnfoOGq3898fKDpOhAilK4btaZcYa9CnNP bzpfPXas4pwv92PBOtOjQ== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeefvddrtddtgdduvdehgeekucetufdoteggodetrf dotffvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdggtfgfnhhsuhgsshgtrhhisggv pdfurfetoffkrfgpnffqhgenuceurghilhhouhhtmecufedttdenucesvcftvggtihhpih gvnhhtshculddquddttddmnecujfgurhephfffufggtgfgkfhfjgfvvefosehtjeertder tdejnecuhfhrohhmpefrrghtrhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhksh drihhmqeenucggtffrrghtthgvrhhnpeffueeiudejvdekheeuvdekfeffiedvueelteek udehjeetkeegvddugfdtgfeileenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmh epmhgrihhlfhhrohhmpehpshesphhkshdrihhmpdhnsggprhgtphhtthhopeejpdhmohgu vgepshhmthhpohhuthdprhgtphhtthhopehshhgvjhhirghluhhosehgmhgrihhlrdgtoh hmpdhrtghpthhtohepghhithhsthgvrhesphhosghogidrtghomhdprhgtphhtthhopegt hhhrihhstghoohhlsehtuhigfhgrmhhilhihrdhorhhgpdhrtghpthhtohepphgvfhhfse hpvghffhdrnhgvthdprhgtphhtthhopehkrghrthhhihhkrddukeeksehgmhgrihhlrdgt ohhmpdhrtghpthhtohepshgrnhgurghlshestghruhhsthihthhoohhthhhprghsthgvrd hnvghtpdhrtghpthhtohepghhithesvhhgvghrrdhkvghrnhgvlhdrohhrgh X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Wed, 12 Mar 2025 11:56:21 -0400 (EDT) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 87f9dcaa (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Wed, 12 Mar 2025 15:56:18 +0000 (UTC) From: Patrick Steinhardt Date: Wed, 12 Mar 2025 16:56:15 +0100 Subject: [PATCH v6 09/16] refs/iterator: separate lifecycle from iteration Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Message-Id: <20250312-pks-update-ref-optimization-v6-9-f778e0414f55@pks.im> References: <20250312-pks-update-ref-optimization-v6-0-f778e0414f55@pks.im> In-Reply-To: <20250312-pks-update-ref-optimization-v6-0-f778e0414f55@pks.im> To: git@vger.kernel.org Cc: Karthik Nayak , "brian m. carlson" , Jeff King , Junio C Hamano , shejialuo , Christian Couder X-Mailer: b4 0.14.2 The ref and reflog iterators have their lifecycle attached to iteration: once the iterator reaches its end, it is automatically released and the caller doesn't have to care about that anymore. When the iterator should be released before it has been exhausted, callers must explicitly abort the iterator via `ref_iterator_abort()`. This lifecycle is somewhat unusual in the Git codebase and creates two problems: - Callsites need to be very careful about when exactly they call `ref_iterator_abort()`, as calling the function is only valid when the iterator itself still is. This leads to somewhat awkward calling patterns in some situations. - It is impossible to reuse iterators and re-seek them to a different prefix. This feature isn't supported by any iterator implementation except for the reftable iterators anyway, but if it was implemented it would allow us to optimize cases where we need to search for specific references repeatedly by reusing internal state. Detangle the lifecycle from iteration so that we don't deallocate the iterator anymore once it is exhausted. Instead, callers are now expected to always call a newly introduce `ref_iterator_free()` function that deallocates the iterator and its internal state. Note that the `dir_iterator` is somewhat special because it does not implement the `ref_iterator` interface, but is only used to implement other iterators. Consequently, we have to provide `dir_iterator_free()` instead of `dir_iterator_release()` as the allocated structure itself is managed by the `dir_iterator` interfaces, as well, and not freed by `ref_iterator_free()` like in all the other cases. While at it, drop the return value of `ref_iterator_abort()`, which wasn't really required by any of the iterator implementations anyway. Furthermore, stop calling `base_ref_iterator_free()` in any of the backends, but instead call it in `ref_iterator_free()`. Signed-off-by: Patrick Steinhardt --- builtin/clone.c | 2 + dir-iterator.c | 24 +++++------ dir-iterator.h | 11 ++--- iterator.h | 2 +- refs.c | 7 ++- refs/debug.c | 9 ++-- refs/files-backend.c | 36 +++++----------- refs/iterator.c | 100 +++++++++++++++---------------------------- refs/packed-backend.c | 27 ++++++------ refs/ref-cache.c | 9 ++-- refs/refs-internal.h | 29 ++++--------- refs/reftable-backend.c | 34 ++++----------- t/helper/test-dir-iterator.c | 1 + 13 files changed, 105 insertions(+), 186 deletions(-) diff --git a/builtin/clone.c b/builtin/clone.c index fd001d800c6..ac3e84b2b18 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -426,6 +426,8 @@ static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest, strbuf_setlen(src, src_len); die(_("failed to iterate over '%s'"), src->buf); } + + dir_iterator_free(iter); } static void clone_local(const char *src_repo, const char *dest_repo) diff --git a/dir-iterator.c b/dir-iterator.c index de619846f29..857e1d9bdaf 100644 --- a/dir-iterator.c +++ b/dir-iterator.c @@ -193,9 +193,9 @@ int dir_iterator_advance(struct dir_iterator *dir_iterator) if (S_ISDIR(iter->base.st.st_mode) && push_level(iter)) { if (errno != ENOENT && iter->flags & DIR_ITERATOR_PEDANTIC) - goto error_out; + return ITER_ERROR; if (iter->levels_nr == 0) - goto error_out; + return ITER_ERROR; } /* Loop until we find an entry that we can give back to the caller. */ @@ -211,11 +211,11 @@ int dir_iterator_advance(struct dir_iterator *dir_iterator) int ret = next_directory_entry(level->dir, iter->base.path.buf, &de); if (ret < 0) { if (iter->flags & DIR_ITERATOR_PEDANTIC) - goto error_out; + return ITER_ERROR; continue; } else if (ret > 0) { if (pop_level(iter) == 0) - return dir_iterator_abort(dir_iterator); + return ITER_DONE; continue; } @@ -223,7 +223,7 @@ int dir_iterator_advance(struct dir_iterator *dir_iterator) } else { if (level->entries_idx >= level->entries.nr) { if (pop_level(iter) == 0) - return dir_iterator_abort(dir_iterator); + return ITER_DONE; continue; } @@ -232,22 +232,21 @@ int dir_iterator_advance(struct dir_iterator *dir_iterator) if (prepare_next_entry_data(iter, name)) { if (errno != ENOENT && iter->flags & DIR_ITERATOR_PEDANTIC) - goto error_out; + return ITER_ERROR; continue; } return ITER_OK; } - -error_out: - dir_iterator_abort(dir_iterator); - return ITER_ERROR; } -int dir_iterator_abort(struct dir_iterator *dir_iterator) +void dir_iterator_free(struct dir_iterator *dir_iterator) { struct dir_iterator_int *iter = (struct dir_iterator_int *)dir_iterator; + if (!iter) + return; + for (; iter->levels_nr; iter->levels_nr--) { struct dir_iterator_level *level = &iter->levels[iter->levels_nr - 1]; @@ -266,7 +265,6 @@ int dir_iterator_abort(struct dir_iterator *dir_iterator) free(iter->levels); strbuf_release(&iter->base.path); free(iter); - return ITER_DONE; } struct dir_iterator *dir_iterator_begin(const char *path, unsigned int flags) @@ -301,7 +299,7 @@ struct dir_iterator *dir_iterator_begin(const char *path, unsigned int flags) return dir_iterator; error_out: - dir_iterator_abort(dir_iterator); + dir_iterator_free(dir_iterator); errno = saved_errno; return NULL; } diff --git a/dir-iterator.h b/dir-iterator.h index 6d438809b6e..ccd6a197343 100644 --- a/dir-iterator.h +++ b/dir-iterator.h @@ -28,7 +28,7 @@ * * while ((ok = dir_iterator_advance(iter)) == ITER_OK) { * if (want_to_stop_iteration()) { - * ok = dir_iterator_abort(iter); + * ok = ITER_DONE; * break; * } * @@ -39,6 +39,7 @@ * * if (ok != ITER_DONE) * handle_error(); + * dir_iterator_free(iter); * * Callers are allowed to modify iter->path while they are working, * but they must restore it to its original contents before calling @@ -107,11 +108,7 @@ struct dir_iterator *dir_iterator_begin(const char *path, unsigned int flags); */ int dir_iterator_advance(struct dir_iterator *iterator); -/* - * End the iteration before it has been exhausted. Free the - * dir_iterator and any associated resources and return ITER_DONE. On - * error, free the dir_iterator and return ITER_ERROR. - */ -int dir_iterator_abort(struct dir_iterator *iterator); +/* Free the dir_iterator and any associated resources. */ +void dir_iterator_free(struct dir_iterator *iterator); #endif diff --git a/iterator.h b/iterator.h index 0f6900e43ad..6b77dcc2626 100644 --- a/iterator.h +++ b/iterator.h @@ -12,7 +12,7 @@ #define ITER_OK 0 /* - * The iterator is exhausted and has been freed. + * The iterator is exhausted. */ #define ITER_DONE -1 diff --git a/refs.c b/refs.c index 111e3cf3aa9..3e65ccad7ac 100644 --- a/refs.c +++ b/refs.c @@ -2477,6 +2477,7 @@ int refs_verify_refnames_available(struct ref_store *refs, struct strbuf dirname = STRBUF_INIT; struct strbuf referent = STRBUF_INIT; struct string_list_item *item; + struct ref_iterator *iter = NULL; struct strset dirnames; int ret = -1; @@ -2553,7 +2554,6 @@ int refs_verify_refnames_available(struct ref_store *refs, strbuf_addch(&dirname, '/'); if (!initial_transaction) { - struct ref_iterator *iter; int ok; iter = refs_ref_iterator_begin(refs, dirname.buf, NULL, 0, @@ -2565,12 +2565,14 @@ int refs_verify_refnames_available(struct ref_store *refs, strbuf_addf(err, _("'%s' exists; cannot create '%s'"), iter->refname, refname); - ref_iterator_abort(iter); goto cleanup; } if (ok != ITER_DONE) BUG("error while iterating over references"); + + ref_iterator_free(iter); + iter = NULL; } extra_refname = find_descendant_ref(dirname.buf, extras, skip); @@ -2587,6 +2589,7 @@ int refs_verify_refnames_available(struct ref_store *refs, strbuf_release(&referent); strbuf_release(&dirname); strset_clear(&dirnames); + ref_iterator_free(iter); return ret; } diff --git a/refs/debug.c b/refs/debug.c index fbc4df08b43..a9786da4ba1 100644 --- a/refs/debug.c +++ b/refs/debug.c @@ -179,19 +179,18 @@ static int debug_ref_iterator_peel(struct ref_iterator *ref_iterator, return res; } -static int debug_ref_iterator_abort(struct ref_iterator *ref_iterator) +static void debug_ref_iterator_release(struct ref_iterator *ref_iterator) { struct debug_ref_iterator *diter = (struct debug_ref_iterator *)ref_iterator; - int res = diter->iter->vtable->abort(diter->iter); - trace_printf_key(&trace_refs, "iterator_abort: %d\n", res); - return res; + diter->iter->vtable->release(diter->iter); + trace_printf_key(&trace_refs, "iterator_abort\n"); } static struct ref_iterator_vtable debug_ref_iterator_vtable = { .advance = debug_ref_iterator_advance, .peel = debug_ref_iterator_peel, - .abort = debug_ref_iterator_abort, + .release = debug_ref_iterator_release, }; static struct ref_iterator * diff --git a/refs/files-backend.c b/refs/files-backend.c index ab6f0af5502..e97a267ad65 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -915,10 +915,6 @@ static int files_ref_iterator_advance(struct ref_iterator *ref_iterator) return ITER_OK; } - iter->iter0 = NULL; - if (ref_iterator_abort(ref_iterator) != ITER_DONE) - ok = ITER_ERROR; - return ok; } @@ -931,23 +927,17 @@ static int files_ref_iterator_peel(struct ref_iterator *ref_iterator, return ref_iterator_peel(iter->iter0, peeled); } -static int files_ref_iterator_abort(struct ref_iterator *ref_iterator) +static void files_ref_iterator_release(struct ref_iterator *ref_iterator) { struct files_ref_iterator *iter = (struct files_ref_iterator *)ref_iterator; - int ok = ITER_DONE; - - if (iter->iter0) - ok = ref_iterator_abort(iter->iter0); - - base_ref_iterator_free(ref_iterator); - return ok; + ref_iterator_free(iter->iter0); } static struct ref_iterator_vtable files_ref_iterator_vtable = { .advance = files_ref_iterator_advance, .peel = files_ref_iterator_peel, - .abort = files_ref_iterator_abort, + .release = files_ref_iterator_release, }; static struct ref_iterator *files_ref_iterator_begin( @@ -1378,7 +1368,7 @@ static int should_pack_refs(struct files_ref_store *refs, iter->flags, opts)) refcount++; if (refcount >= limit) { - ref_iterator_abort(iter); + ref_iterator_free(iter); return 1; } } @@ -1386,6 +1376,7 @@ static int should_pack_refs(struct files_ref_store *refs, if (ret != ITER_DONE) die("error while iterating over references"); + ref_iterator_free(iter); return 0; } @@ -1452,6 +1443,7 @@ static int files_pack_refs(struct ref_store *ref_store, packed_refs_unlock(refs->packed_ref_store); prune_refs(refs, &refs_to_prune); + ref_iterator_free(iter); strbuf_release(&err); return 0; } @@ -2299,9 +2291,6 @@ static int files_reflog_iterator_advance(struct ref_iterator *ref_iterator) return ITER_OK; } - iter->dir_iterator = NULL; - if (ref_iterator_abort(ref_iterator) == ITER_ERROR) - ok = ITER_ERROR; return ok; } @@ -2311,23 +2300,17 @@ static int files_reflog_iterator_peel(struct ref_iterator *ref_iterator UNUSED, BUG("ref_iterator_peel() called for reflog_iterator"); } -static int files_reflog_iterator_abort(struct ref_iterator *ref_iterator) +static void files_reflog_iterator_release(struct ref_iterator *ref_iterator) { struct files_reflog_iterator *iter = (struct files_reflog_iterator *)ref_iterator; - int ok = ITER_DONE; - - if (iter->dir_iterator) - ok = dir_iterator_abort(iter->dir_iterator); - - base_ref_iterator_free(ref_iterator); - return ok; + dir_iterator_free(iter->dir_iterator); } static struct ref_iterator_vtable files_reflog_iterator_vtable = { .advance = files_reflog_iterator_advance, .peel = files_reflog_iterator_peel, - .abort = files_reflog_iterator_abort, + .release = files_reflog_iterator_release, }; static struct ref_iterator *reflog_iterator_begin(struct ref_store *ref_store, @@ -3837,6 +3820,7 @@ static int files_fsck_refs_dir(struct ref_store *ref_store, ret = error(_("failed to iterate over '%s'"), sb.buf); out: + dir_iterator_free(iter); strbuf_release(&sb); strbuf_release(&refname); return ret; diff --git a/refs/iterator.c b/refs/iterator.c index d25e568bf0b..d61474cba75 100644 --- a/refs/iterator.c +++ b/refs/iterator.c @@ -21,9 +21,14 @@ int ref_iterator_peel(struct ref_iterator *ref_iterator, return ref_iterator->vtable->peel(ref_iterator, peeled); } -int ref_iterator_abort(struct ref_iterator *ref_iterator) +void ref_iterator_free(struct ref_iterator *ref_iterator) { - return ref_iterator->vtable->abort(ref_iterator); + if (ref_iterator) { + ref_iterator->vtable->release(ref_iterator); + /* Help make use-after-free bugs fail quickly: */ + ref_iterator->vtable = NULL; + free(ref_iterator); + } } void base_ref_iterator_init(struct ref_iterator *iter, @@ -36,20 +41,13 @@ void base_ref_iterator_init(struct ref_iterator *iter, iter->flags = 0; } -void base_ref_iterator_free(struct ref_iterator *iter) -{ - /* Help make use-after-free bugs fail quickly: */ - iter->vtable = NULL; - free(iter); -} - struct empty_ref_iterator { struct ref_iterator base; }; -static int empty_ref_iterator_advance(struct ref_iterator *ref_iterator) +static int empty_ref_iterator_advance(struct ref_iterator *ref_iterator UNUSED) { - return ref_iterator_abort(ref_iterator); + return ITER_DONE; } static int empty_ref_iterator_peel(struct ref_iterator *ref_iterator UNUSED, @@ -58,16 +56,14 @@ static int empty_ref_iterator_peel(struct ref_iterator *ref_iterator UNUSED, BUG("peel called for empty iterator"); } -static int empty_ref_iterator_abort(struct ref_iterator *ref_iterator) +static void empty_ref_iterator_release(struct ref_iterator *ref_iterator UNUSED) { - base_ref_iterator_free(ref_iterator); - return ITER_DONE; } static struct ref_iterator_vtable empty_ref_iterator_vtable = { .advance = empty_ref_iterator_advance, .peel = empty_ref_iterator_peel, - .abort = empty_ref_iterator_abort, + .release = empty_ref_iterator_release, }; struct ref_iterator *empty_ref_iterator_begin(void) @@ -151,11 +147,13 @@ static int merge_ref_iterator_advance(struct ref_iterator *ref_iterator) if (!iter->current) { /* Initialize: advance both iterators to their first entries */ if ((ok = ref_iterator_advance(iter->iter0)) != ITER_OK) { + ref_iterator_free(iter->iter0); iter->iter0 = NULL; if (ok == ITER_ERROR) goto error; } if ((ok = ref_iterator_advance(iter->iter1)) != ITER_OK) { + ref_iterator_free(iter->iter1); iter->iter1 = NULL; if (ok == ITER_ERROR) goto error; @@ -166,6 +164,7 @@ static int merge_ref_iterator_advance(struct ref_iterator *ref_iterator) * entry: */ if ((ok = ref_iterator_advance(*iter->current)) != ITER_OK) { + ref_iterator_free(*iter->current); *iter->current = NULL; if (ok == ITER_ERROR) goto error; @@ -179,9 +178,8 @@ static int merge_ref_iterator_advance(struct ref_iterator *ref_iterator) iter->select(iter->iter0, iter->iter1, iter->cb_data); if (selection == ITER_SELECT_DONE) { - return ref_iterator_abort(ref_iterator); + return ITER_DONE; } else if (selection == ITER_SELECT_ERROR) { - ref_iterator_abort(ref_iterator); return ITER_ERROR; } @@ -195,6 +193,7 @@ static int merge_ref_iterator_advance(struct ref_iterator *ref_iterator) if (selection & ITER_SKIP_SECONDARY) { if ((ok = ref_iterator_advance(*secondary)) != ITER_OK) { + ref_iterator_free(*secondary); *secondary = NULL; if (ok == ITER_ERROR) goto error; @@ -211,7 +210,6 @@ static int merge_ref_iterator_advance(struct ref_iterator *ref_iterator) } error: - ref_iterator_abort(ref_iterator); return ITER_ERROR; } @@ -227,28 +225,18 @@ static int merge_ref_iterator_peel(struct ref_iterator *ref_iterator, return ref_iterator_peel(*iter->current, peeled); } -static int merge_ref_iterator_abort(struct ref_iterator *ref_iterator) +static void merge_ref_iterator_release(struct ref_iterator *ref_iterator) { struct merge_ref_iterator *iter = (struct merge_ref_iterator *)ref_iterator; - int ok = ITER_DONE; - - if (iter->iter0) { - if (ref_iterator_abort(iter->iter0) != ITER_DONE) - ok = ITER_ERROR; - } - if (iter->iter1) { - if (ref_iterator_abort(iter->iter1) != ITER_DONE) - ok = ITER_ERROR; - } - base_ref_iterator_free(ref_iterator); - return ok; + ref_iterator_free(iter->iter0); + ref_iterator_free(iter->iter1); } static struct ref_iterator_vtable merge_ref_iterator_vtable = { .advance = merge_ref_iterator_advance, .peel = merge_ref_iterator_peel, - .abort = merge_ref_iterator_abort, + .release = merge_ref_iterator_release, }; struct ref_iterator *merge_ref_iterator_begin( @@ -310,10 +298,10 @@ struct ref_iterator *overlay_ref_iterator_begin( * them. */ if (is_empty_ref_iterator(front)) { - ref_iterator_abort(front); + ref_iterator_free(front); return back; } else if (is_empty_ref_iterator(back)) { - ref_iterator_abort(back); + ref_iterator_free(back); return front; } @@ -350,19 +338,15 @@ static int prefix_ref_iterator_advance(struct ref_iterator *ref_iterator) while ((ok = ref_iterator_advance(iter->iter0)) == ITER_OK) { int cmp = compare_prefix(iter->iter0->refname, iter->prefix); - if (cmp < 0) continue; - - if (cmp > 0) { - /* - * As the source iterator is ordered, we - * can stop the iteration as soon as we see a - * refname that comes after the prefix: - */ - ok = ref_iterator_abort(iter->iter0); - break; - } + /* + * As the source iterator is ordered, we + * can stop the iteration as soon as we see a + * refname that comes after the prefix: + */ + if (cmp > 0) + return ITER_DONE; if (iter->trim) { /* @@ -386,9 +370,6 @@ static int prefix_ref_iterator_advance(struct ref_iterator *ref_iterator) return ITER_OK; } - iter->iter0 = NULL; - if (ref_iterator_abort(ref_iterator) != ITER_DONE) - return ITER_ERROR; return ok; } @@ -401,23 +382,18 @@ static int prefix_ref_iterator_peel(struct ref_iterator *ref_iterator, return ref_iterator_peel(iter->iter0, peeled); } -static int prefix_ref_iterator_abort(struct ref_iterator *ref_iterator) +static void prefix_ref_iterator_release(struct ref_iterator *ref_iterator) { struct prefix_ref_iterator *iter = (struct prefix_ref_iterator *)ref_iterator; - int ok = ITER_DONE; - - if (iter->iter0) - ok = ref_iterator_abort(iter->iter0); + ref_iterator_free(iter->iter0); free(iter->prefix); - base_ref_iterator_free(ref_iterator); - return ok; } static struct ref_iterator_vtable prefix_ref_iterator_vtable = { .advance = prefix_ref_iterator_advance, .peel = prefix_ref_iterator_peel, - .abort = prefix_ref_iterator_abort, + .release = prefix_ref_iterator_release, }; struct ref_iterator *prefix_ref_iterator_begin(struct ref_iterator *iter0, @@ -453,20 +429,14 @@ int do_for_each_ref_iterator(struct ref_iterator *iter, current_ref_iter = iter; while ((ok = ref_iterator_advance(iter)) == ITER_OK) { retval = fn(iter->refname, iter->referent, iter->oid, iter->flags, cb_data); - if (retval) { - /* - * If ref_iterator_abort() returns ITER_ERROR, - * we ignore that error in deference to the - * callback function's return value. - */ - ref_iterator_abort(iter); + if (retval) goto out; - } } out: current_ref_iter = old_ref_iter; if (ok == ITER_ERROR) - return -1; + retval = -1; + ref_iterator_free(iter); return retval; } diff --git a/refs/packed-backend.c b/refs/packed-backend.c index a7b6f74b6e3..38a1956d1a8 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -954,9 +954,6 @@ static int packed_ref_iterator_advance(struct ref_iterator *ref_iterator) return ITER_OK; } - if (ref_iterator_abort(ref_iterator) != ITER_DONE) - ok = ITER_ERROR; - return ok; } @@ -976,23 +973,19 @@ static int packed_ref_iterator_peel(struct ref_iterator *ref_iterator, } } -static int packed_ref_iterator_abort(struct ref_iterator *ref_iterator) +static void packed_ref_iterator_release(struct ref_iterator *ref_iterator) { struct packed_ref_iterator *iter = (struct packed_ref_iterator *)ref_iterator; - int ok = ITER_DONE; - strbuf_release(&iter->refname_buf); free(iter->jump); release_snapshot(iter->snapshot); - base_ref_iterator_free(ref_iterator); - return ok; } static struct ref_iterator_vtable packed_ref_iterator_vtable = { .advance = packed_ref_iterator_advance, .peel = packed_ref_iterator_peel, - .abort = packed_ref_iterator_abort + .release = packed_ref_iterator_release, }; static int jump_list_entry_cmp(const void *va, const void *vb) @@ -1362,8 +1355,10 @@ static int write_with_updates(struct packed_ref_store *refs, */ iter = packed_ref_iterator_begin(&refs->base, "", NULL, DO_FOR_EACH_INCLUDE_BROKEN); - if ((ok = ref_iterator_advance(iter)) != ITER_OK) + if ((ok = ref_iterator_advance(iter)) != ITER_OK) { + ref_iterator_free(iter); iter = NULL; + } i = 0; @@ -1411,8 +1406,10 @@ static int write_with_updates(struct packed_ref_store *refs, * the iterator over the unneeded * value. */ - if ((ok = ref_iterator_advance(iter)) != ITER_OK) + if ((ok = ref_iterator_advance(iter)) != ITER_OK) { + ref_iterator_free(iter); iter = NULL; + } cmp = +1; } else { /* @@ -1449,8 +1446,10 @@ static int write_with_updates(struct packed_ref_store *refs, peel_error ? NULL : &peeled)) goto write_error; - if ((ok = ref_iterator_advance(iter)) != ITER_OK) + if ((ok = ref_iterator_advance(iter)) != ITER_OK) { + ref_iterator_free(iter); iter = NULL; + } } else if (is_null_oid(&update->new_oid)) { /* * The update wants to delete the reference, @@ -1499,9 +1498,7 @@ static int write_with_updates(struct packed_ref_store *refs, get_tempfile_path(refs->tempfile), strerror(errno)); error: - if (iter) - ref_iterator_abort(iter); - + ref_iterator_free(iter); delete_tempfile(&refs->tempfile); return -1; } diff --git a/refs/ref-cache.c b/refs/ref-cache.c index 02f09e4df88..6457e02c1ea 100644 --- a/refs/ref-cache.c +++ b/refs/ref-cache.c @@ -409,7 +409,7 @@ static int cache_ref_iterator_advance(struct ref_iterator *ref_iterator) if (++level->index == level->dir->nr) { /* This level is exhausted; pop up a level */ if (--iter->levels_nr == 0) - return ref_iterator_abort(ref_iterator); + return ITER_DONE; continue; } @@ -452,21 +452,18 @@ static int cache_ref_iterator_peel(struct ref_iterator *ref_iterator, return peel_object(iter->repo, ref_iterator->oid, peeled) ? -1 : 0; } -static int cache_ref_iterator_abort(struct ref_iterator *ref_iterator) +static void cache_ref_iterator_release(struct ref_iterator *ref_iterator) { struct cache_ref_iterator *iter = (struct cache_ref_iterator *)ref_iterator; - free((char *)iter->prefix); free(iter->levels); - base_ref_iterator_free(ref_iterator); - return ITER_DONE; } static struct ref_iterator_vtable cache_ref_iterator_vtable = { .advance = cache_ref_iterator_advance, .peel = cache_ref_iterator_peel, - .abort = cache_ref_iterator_abort + .release = cache_ref_iterator_release, }; struct ref_iterator *cache_ref_iterator_begin(struct ref_cache *cache, diff --git a/refs/refs-internal.h b/refs/refs-internal.h index aaab711bb96..74e2c03cef1 100644 --- a/refs/refs-internal.h +++ b/refs/refs-internal.h @@ -273,11 +273,11 @@ enum do_for_each_ref_flags { * the next reference and returns ITER_OK. The data pointed at by * refname and oid belong to the iterator; if you want to retain them * after calling ref_iterator_advance() again or calling - * ref_iterator_abort(), you must make a copy. When the iteration has + * ref_iterator_free(), you must make a copy. When the iteration has * been exhausted, ref_iterator_advance() releases any resources * associated with the iteration, frees the ref_iterator object, and * returns ITER_DONE. If you want to abort the iteration early, call - * ref_iterator_abort(), which also frees the ref_iterator object and + * ref_iterator_free(), which also frees the ref_iterator object and * any associated resources. If there was an internal error advancing * to the next entry, ref_iterator_advance() aborts the iteration, * frees the ref_iterator, and returns ITER_ERROR. @@ -293,7 +293,7 @@ enum do_for_each_ref_flags { * * while ((ok = ref_iterator_advance(iter)) == ITER_OK) { * if (want_to_stop_iteration()) { - * ok = ref_iterator_abort(iter); + * ok = ITER_DONE; * break; * } * @@ -307,6 +307,7 @@ enum do_for_each_ref_flags { * * if (ok != ITER_DONE) * handle_error(); + * ref_iterator_free(iter); */ struct ref_iterator { struct ref_iterator_vtable *vtable; @@ -333,12 +334,8 @@ int ref_iterator_advance(struct ref_iterator *ref_iterator); int ref_iterator_peel(struct ref_iterator *ref_iterator, struct object_id *peeled); -/* - * End the iteration before it has been exhausted, freeing the - * reference iterator and any associated resources and returning - * ITER_DONE. If the abort itself failed, return ITER_ERROR. - */ -int ref_iterator_abort(struct ref_iterator *ref_iterator); +/* Free the reference iterator and any associated resources. */ +void ref_iterator_free(struct ref_iterator *ref_iterator); /* * An iterator over nothing (its first ref_iterator_advance() call @@ -438,13 +435,6 @@ struct ref_iterator *prefix_ref_iterator_begin(struct ref_iterator *iter0, void base_ref_iterator_init(struct ref_iterator *iter, struct ref_iterator_vtable *vtable); -/* - * Base class destructor for ref_iterators. Destroy the ref_iterator - * part of iter and shallow-free the object. This is meant to be - * called only by the destructors of derived classes. - */ -void base_ref_iterator_free(struct ref_iterator *iter); - /* Virtual function declarations for ref_iterators: */ /* @@ -463,15 +453,14 @@ typedef int ref_iterator_peel_fn(struct ref_iterator *ref_iterator, /* * Implementations of this function should free any resources specific - * to the derived class, then call base_ref_iterator_free() to clean - * up and free the ref_iterator object. + * to the derived class. */ -typedef int ref_iterator_abort_fn(struct ref_iterator *ref_iterator); +typedef void ref_iterator_release_fn(struct ref_iterator *ref_iterator); struct ref_iterator_vtable { ref_iterator_advance_fn *advance; ref_iterator_peel_fn *peel; - ref_iterator_abort_fn *abort; + ref_iterator_release_fn *release; }; /* diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c index 5c464b9d143..57d8512fe80 100644 --- a/refs/reftable-backend.c +++ b/refs/reftable-backend.c @@ -711,17 +711,10 @@ static int reftable_ref_iterator_advance(struct ref_iterator *ref_iterator) break; } - if (iter->err > 0) { - if (ref_iterator_abort(ref_iterator) != ITER_DONE) - return ITER_ERROR; + if (iter->err > 0) return ITER_DONE; - } - - if (iter->err < 0) { - ref_iterator_abort(ref_iterator); + if (iter->err < 0) return ITER_ERROR; - } - return ITER_OK; } @@ -740,7 +733,7 @@ static int reftable_ref_iterator_peel(struct ref_iterator *ref_iterator, return -1; } -static int reftable_ref_iterator_abort(struct ref_iterator *ref_iterator) +static void reftable_ref_iterator_release(struct ref_iterator *ref_iterator) { struct reftable_ref_iterator *iter = (struct reftable_ref_iterator *)ref_iterator; @@ -751,14 +744,12 @@ static int reftable_ref_iterator_abort(struct ref_iterator *ref_iterator) free(iter->exclude_patterns[i]); free(iter->exclude_patterns); } - free(iter); - return ITER_DONE; } static struct ref_iterator_vtable reftable_ref_iterator_vtable = { .advance = reftable_ref_iterator_advance, .peel = reftable_ref_iterator_peel, - .abort = reftable_ref_iterator_abort + .release = reftable_ref_iterator_release, }; static int qsort_strcmp(const void *va, const void *vb) @@ -2016,17 +2007,10 @@ static int reftable_reflog_iterator_advance(struct ref_iterator *ref_iterator) break; } - if (iter->err > 0) { - if (ref_iterator_abort(ref_iterator) != ITER_DONE) - return ITER_ERROR; + if (iter->err > 0) return ITER_DONE; - } - - if (iter->err < 0) { - ref_iterator_abort(ref_iterator); + if (iter->err < 0) return ITER_ERROR; - } - return ITER_OK; } @@ -2037,21 +2021,19 @@ static int reftable_reflog_iterator_peel(struct ref_iterator *ref_iterator UNUSE return -1; } -static int reftable_reflog_iterator_abort(struct ref_iterator *ref_iterator) +static void reftable_reflog_iterator_release(struct ref_iterator *ref_iterator) { struct reftable_reflog_iterator *iter = (struct reftable_reflog_iterator *)ref_iterator; reftable_log_record_release(&iter->log); reftable_iterator_destroy(&iter->iter); strbuf_release(&iter->last_name); - free(iter); - return ITER_DONE; } static struct ref_iterator_vtable reftable_reflog_iterator_vtable = { .advance = reftable_reflog_iterator_advance, .peel = reftable_reflog_iterator_peel, - .abort = reftable_reflog_iterator_abort + .release = reftable_reflog_iterator_release, }; static struct reftable_reflog_iterator *reflog_iterator_for_stack(struct reftable_ref_store *refs, diff --git a/t/helper/test-dir-iterator.c b/t/helper/test-dir-iterator.c index 6b297bd7536..8d46e8ba409 100644 --- a/t/helper/test-dir-iterator.c +++ b/t/helper/test-dir-iterator.c @@ -53,6 +53,7 @@ int cmd__dir_iterator(int argc, const char **argv) printf("(%s) [%s] %s\n", diter->relative_path, diter->basename, diter->path.buf); } + dir_iterator_free(diter); if (iter_status != ITER_DONE) { printf("dir_iterator_advance failure\n"); From patchwork Wed Mar 12 15:56:16 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 14013665 Received: from fhigh-b4-smtp.messagingengine.com (fhigh-b4-smtp.messagingengine.com [202.12.124.155]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 754F3258CC2 for ; Wed, 12 Mar 2025 15:56:24 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=202.12.124.155 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741794986; cv=none; b=eAS1VCVCmT6KlPd9p9yqEGojEP4W+F1pAwoK670izqaz/ZWEi40zIecRyjL09PpftnnEMyeRwubhevkdz/1aj9r3MJm0yZhTDoR1wvGppkhbpZo1zfNqfBPBha0if4RSJW085C95LXAl03QneVgpAwf/p3vPd/CViqIQtdsyFYk= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741794986; c=relaxed/simple; bh=clLnuC4xOE2mwWGZsGSbHv7SfzpE3gPHxEJYIQPgJHs=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:References: In-Reply-To:To:Cc; b=I1kE+R4zG0H5V8bqhkhFb5ZXsvUvCz/fV3D4+38Iiu912nIIwc2XVm70jDCMVLM34CwjaJFNW3Zv7P/0Gulbm+jY/KOLJxNNN1C4GbjRxVJYycPfDAm0BefSsJqUB6hCmPMmY3XKipXrvkdy3PlNgalAw9xXQvqV5VgkxjlbQSE= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im; spf=pass smtp.mailfrom=pks.im; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b=JsdyEa6X; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=JRGirDzs; arc=none smtp.client-ip=202.12.124.155 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pks.im Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="JsdyEa6X"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="JRGirDzs" Received: from phl-compute-01.internal (phl-compute-01.phl.internal [10.202.2.41]) by mailfhigh.stl.internal (Postfix) with ESMTP id 7440A254030A; Wed, 12 Mar 2025 11:56:23 -0400 (EDT) Received: from phl-mailfrontend-01 ([10.202.2.162]) by phl-compute-01.internal (MEProxy); Wed, 12 Mar 2025 11:56:23 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :content-transfer-encoding:content-type:content-type:date:date :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to; s=fm1; t=1741794983; x=1741881383; bh=MPkifl6idFsDZJDfinOJQ5Gy8gxFzCeZeqRZeEF8W5o=; b= JsdyEa6X0e/P0xgO2qMGHvqFRuxFqKuvJBe2kohNq3eD24nBxqj7jmNtfTcMZAGc tQ0cKonevwFFbM2jANDHCBfbazm+wqOh/y+cfLbEgdmNHd0I+a12KAFgRZ5H+Wjr qLPDwlWkEUBfeRsRXpv8u2l87zOm+Q4Z3tSWlYO4vMytQRkI8SnswvAvj4OxOyto bNS2UEHEy5c22UBJcyY/oIYbObBU2gfR5NH+OF6BiZxn5fTszlbZJ3NZMB2cnqcv HDaPvuVA4BpUkjyqEzlWSC8AerjmLuD1uRJ4sInBctCBQMPQLlg3iuSOtVlkOxC4 Z0O6dq+Ar2YVg7/ZHHMg3Q== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:content-type:date:date:feedback-id:feedback-id :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to:x-me-proxy :x-me-sender:x-me-sender:x-sasl-enc; s=fm1; t=1741794983; x= 1741881383; bh=MPkifl6idFsDZJDfinOJQ5Gy8gxFzCeZeqRZeEF8W5o=; b=J RGirDzsOtbB2QjJ3/PE9NAr6evysOkzbOhFdEHqKUDxrjs/N45PM5BO6vE2qzSaa BhNR+INd/nrMk66hkh2OR4PX7hfC9AvYnqwiVoXbBSWDTqRH703/ks+r6QXN3Cnw NzjVwrFnIWKhg2M28wK/VEpOzJSCrIi3qvOiky91Np6LBUi/5cBN8kv8U7FkEg/y 2GLQMlJLFDGi5jbFI7eN7qg8Yuw4NhTJi2BBd+3tPAcCk4emQKOAgdvhJspNs1XY MSuctr1f8caIm005dXZsKq/sbKiQneO/+hk9MAIKGFM8xb7puTsT9myq1tX02ufl PtgoOZ5WN8HD+H087tukw== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeefvddrtddtgdduvdehgeekucetufdoteggodetrf dotffvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdggtfgfnhhsuhgsshgtrhhisggv pdfurfetoffkrfgpnffqhgenuceurghilhhouhhtmecufedttdenucesvcftvggtihhpih gvnhhtshculddquddttddmnecujfgurhephfffufggtgfgkfhfjgfvvefosehtjeertder tdejnecuhfhrohhmpefrrghtrhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhksh drihhmqeenucggtffrrghtthgvrhhnpeffueeiudejvdekheeuvdekfeffiedvueelteek udehjeetkeegvddugfdtgfeileenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmh epmhgrihhlfhhrohhmpehpshesphhkshdrihhmpdhnsggprhgtphhtthhopeejpdhmohgu vgepshhmthhpohhuthdprhgtphhtthhopehshhgvjhhirghluhhosehgmhgrihhlrdgtoh hmpdhrtghpthhtohepghhithesvhhgvghrrdhkvghrnhgvlhdrohhrghdprhgtphhtthho pegthhhrihhstghoohhlsehtuhigfhgrmhhilhihrdhorhhgpdhrtghpthhtohepghhith hsthgvrhesphhosghogidrtghomhdprhgtphhtthhopehkrghrthhhihhkrddukeeksehg mhgrihhlrdgtohhmpdhrtghpthhtohepshgrnhgurghlshestghruhhsthihthhoohhthh hprghsthgvrdhnvghtpdhrtghpthhtohepphgvfhhfsehpvghffhdrnhgvth X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Wed, 12 Mar 2025 11:56:21 -0400 (EDT) Received: by vm-mail (OpenSMTPD) with ESMTPSA id c86ed10b (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Wed, 12 Mar 2025 15:56:19 +0000 (UTC) From: Patrick Steinhardt Date: Wed, 12 Mar 2025 16:56:16 +0100 Subject: [PATCH v6 10/16] refs/iterator: provide infrastructure to re-seek iterators Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Message-Id: <20250312-pks-update-ref-optimization-v6-10-f778e0414f55@pks.im> References: <20250312-pks-update-ref-optimization-v6-0-f778e0414f55@pks.im> In-Reply-To: <20250312-pks-update-ref-optimization-v6-0-f778e0414f55@pks.im> To: git@vger.kernel.org Cc: Karthik Nayak , "brian m. carlson" , Jeff King , Junio C Hamano , shejialuo , Christian Couder X-Mailer: b4 0.14.2 Reftable iterators need to be scrapped after they have either been exhausted or aren't useful to the caller anymore, and it is explicitly not possible to reuse them for iterations. But enabling for reuse of iterators may allow us to tune them by reusing internal state of an iterator. The reftable iterators for example can already be reused internally, but we're not able to expose this to any users outside of the reftable backend. Introduce a new `.seek` function in the ref iterator vtable that allows callers to seek an iterator multiple times. It is expected to be functionally the same as calling `refs_ref_iterator_begin()` with a different (or the same) prefix. Note that it is not possible to adjust parameters other than the seeked prefix for now, so exclude patterns, trimmed prefixes and flags will remain unchanged. We do not have a usecase for changing these parameters right now, but if we ever find one we can adapt accordingly. Implement the callback for trivial cases. The other iterators will be implemented in subsequent commits. Signed-off-by: Patrick Steinhardt --- refs/debug.c | 11 +++++++++++ refs/iterator.c | 24 ++++++++++++++++++++++++ refs/refs-internal.h | 24 ++++++++++++++++++++++++ 3 files changed, 59 insertions(+) diff --git a/refs/debug.c b/refs/debug.c index a9786da4ba1..5390fa9c187 100644 --- a/refs/debug.c +++ b/refs/debug.c @@ -169,6 +169,16 @@ static int debug_ref_iterator_advance(struct ref_iterator *ref_iterator) return res; } +static int debug_ref_iterator_seek(struct ref_iterator *ref_iterator, + const char *prefix) +{ + struct debug_ref_iterator *diter = + (struct debug_ref_iterator *)ref_iterator; + int res = diter->iter->vtable->seek(diter->iter, prefix); + trace_printf_key(&trace_refs, "iterator_seek: %s: %d\n", prefix ? prefix : "", res); + return res; +} + static int debug_ref_iterator_peel(struct ref_iterator *ref_iterator, struct object_id *peeled) { @@ -189,6 +199,7 @@ static void debug_ref_iterator_release(struct ref_iterator *ref_iterator) static struct ref_iterator_vtable debug_ref_iterator_vtable = { .advance = debug_ref_iterator_advance, + .seek = debug_ref_iterator_seek, .peel = debug_ref_iterator_peel, .release = debug_ref_iterator_release, }; diff --git a/refs/iterator.c b/refs/iterator.c index d61474cba75..ea4db59481d 100644 --- a/refs/iterator.c +++ b/refs/iterator.c @@ -15,6 +15,12 @@ int ref_iterator_advance(struct ref_iterator *ref_iterator) return ref_iterator->vtable->advance(ref_iterator); } +int ref_iterator_seek(struct ref_iterator *ref_iterator, + const char *prefix) +{ + return ref_iterator->vtable->seek(ref_iterator, prefix); +} + int ref_iterator_peel(struct ref_iterator *ref_iterator, struct object_id *peeled) { @@ -50,6 +56,12 @@ static int empty_ref_iterator_advance(struct ref_iterator *ref_iterator UNUSED) return ITER_DONE; } +static int empty_ref_iterator_seek(struct ref_iterator *ref_iterator UNUSED, + const char *prefix UNUSED) +{ + return 0; +} + static int empty_ref_iterator_peel(struct ref_iterator *ref_iterator UNUSED, struct object_id *peeled UNUSED) { @@ -62,6 +74,7 @@ static void empty_ref_iterator_release(struct ref_iterator *ref_iterator UNUSED) static struct ref_iterator_vtable empty_ref_iterator_vtable = { .advance = empty_ref_iterator_advance, + .seek = empty_ref_iterator_seek, .peel = empty_ref_iterator_peel, .release = empty_ref_iterator_release, }; @@ -373,6 +386,16 @@ static int prefix_ref_iterator_advance(struct ref_iterator *ref_iterator) return ok; } +static int prefix_ref_iterator_seek(struct ref_iterator *ref_iterator, + const char *prefix) +{ + struct prefix_ref_iterator *iter = + (struct prefix_ref_iterator *)ref_iterator; + free(iter->prefix); + iter->prefix = xstrdup_or_null(prefix); + return ref_iterator_seek(iter->iter0, prefix); +} + static int prefix_ref_iterator_peel(struct ref_iterator *ref_iterator, struct object_id *peeled) { @@ -392,6 +415,7 @@ static void prefix_ref_iterator_release(struct ref_iterator *ref_iterator) static struct ref_iterator_vtable prefix_ref_iterator_vtable = { .advance = prefix_ref_iterator_advance, + .seek = prefix_ref_iterator_seek, .peel = prefix_ref_iterator_peel, .release = prefix_ref_iterator_release, }; diff --git a/refs/refs-internal.h b/refs/refs-internal.h index 74e2c03cef1..8f18274a165 100644 --- a/refs/refs-internal.h +++ b/refs/refs-internal.h @@ -327,6 +327,22 @@ struct ref_iterator { */ int ref_iterator_advance(struct ref_iterator *ref_iterator); +/* + * Seek the iterator to the first reference with the given prefix. + * The prefix is matched as a literal string, without regard for path + * separators. If prefix is NULL or the empty string, seek the iterator to the + * first reference again. + * + * This function is expected to behave as if a new ref iterator with the same + * prefix had been created, but allows reuse of iterators and thus may allow + * the backend to optimize. Parameters other than the prefix that have been + * passed when creating the iterator will remain unchanged. + * + * Returns 0 on success, a negative error code otherwise. + */ +int ref_iterator_seek(struct ref_iterator *ref_iterator, + const char *prefix); + /* * If possible, peel the reference currently being viewed by the * iterator. Return 0 on success. @@ -445,6 +461,13 @@ void base_ref_iterator_init(struct ref_iterator *iter, */ typedef int ref_iterator_advance_fn(struct ref_iterator *ref_iterator); +/* + * Seek the iterator to the first reference matching the given prefix. Should + * behave the same as if a new iterator was created with the same prefix. + */ +typedef int ref_iterator_seek_fn(struct ref_iterator *ref_iterator, + const char *prefix); + /* * Peels the current ref, returning 0 for success or -1 for failure. */ @@ -459,6 +482,7 @@ typedef void ref_iterator_release_fn(struct ref_iterator *ref_iterator); struct ref_iterator_vtable { ref_iterator_advance_fn *advance; + ref_iterator_seek_fn *seek; ref_iterator_peel_fn *peel; ref_iterator_release_fn *release; }; From patchwork Wed Mar 12 15:56:17 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 14013664 Received: from fhigh-b4-smtp.messagingengine.com (fhigh-b4-smtp.messagingengine.com [202.12.124.155]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 790D1258CC4 for ; Wed, 12 Mar 2025 15:56:24 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=202.12.124.155 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741794986; cv=none; b=AcUw0Z6ndsF70mD7K1eu1my543+mDA1ZC1lDtjPMB6xewxNvdK7CUy0q9meSgDsDRrHCVcx7PTn+UyeHA497pqwtKWYvkFZ+qPLhyPBRocKfvgLsn/HHbBUpGCj/zM7M+EdHLKecNArR0Td1BX8QOtdDjtGRJsknb6yum1q0+GM= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741794986; c=relaxed/simple; bh=evGhBHEShvx+Wz2OMlhodH3XxHSprNJWfhaFexf0qFc=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:References: In-Reply-To:To:Cc; b=sY8hx7DdaYU6yTZPg28XHMsuYioXpLwL+2y8XolA8enQgtWU8y5VVSJNAdEUdgN/ikdVJSaR4QgkGHtdAsmeT1pAC5VTYg7LO+baXYyUAJdXJpqq5fBD/kWEayrpQOokdnEJGG7OPWt/XIZJ4FBwYsJeQZsmy0U1Ej+syQ0K3NI= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im; spf=pass smtp.mailfrom=pks.im; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b=hhKl3150; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=1YUqBsu8; arc=none smtp.client-ip=202.12.124.155 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pks.im Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="hhKl3150"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="1YUqBsu8" Received: from phl-compute-13.internal (phl-compute-13.phl.internal [10.202.2.53]) by mailfhigh.stl.internal (Postfix) with ESMTP id AAA01254030F; Wed, 12 Mar 2025 11:56:23 -0400 (EDT) Received: from phl-mailfrontend-02 ([10.202.2.163]) by phl-compute-13.internal (MEProxy); Wed, 12 Mar 2025 11:56:23 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :content-transfer-encoding:content-type:content-type:date:date :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to; s=fm1; t=1741794983; x=1741881383; bh=3DdDV4H8cwIMEb61hTghlIJjMifNGzs3Dki56QedE+M=; b= hhKl3150dkKogbkA33+umxk81vl+UA8L7k8bhCwxTeckfExjwwjp3gsUedblvjh1 F62aqyoyIEhLPoqj69z0G9A3bud2QPkLBYKqrYUm+GrWrNFoy+gT7CxUnb3ym14l RIk7zGrd8NMZGxz8A6m/mAGQqOofe5xo2y4xmFXHR9BTBlCutEoYbMF5a5z5oI9N CIZZEEZCJLBpqdYtODxrWIZ0XqXffgL1WejcQobGDmiCfYdvUgOeHw2v7ukuWmXb qzRH/0H+icDF9dGrM3L8Fxkmb3CDv/DJNhBuW+7eEsDJ96WFMa5jLJeLRxXAGGYp QnLG3EZjMfglUCyRgO581Q== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:content-type:date:date:feedback-id:feedback-id :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to:x-me-proxy :x-me-sender:x-me-sender:x-sasl-enc; s=fm1; t=1741794983; x= 1741881383; bh=3DdDV4H8cwIMEb61hTghlIJjMifNGzs3Dki56QedE+M=; b=1 YUqBsu8SA1R14HEGaTOJiC03Vrw37ZfiPNdokDaVWn6hnciiMJrAqdTA8iOBDDE5 LwqRAKpT525sD24AA0/P5QSbS8wFNUBlCfHwdj3JZ1nMArocNbNUxzB3mtHuaX0f fl3++CtwDGwbPchPMA/yNAwBNjwece4k9AJPH0+hfSCvVFFE/aNUyrkH6CD1nsPT P1dUwIVX97eifBHnt4WaZO1S9JCMouUvCc7kp6nkxFtUm4ENpk2Yf75qTnHSqtln BNXVElJc+YYqJhldh/+AaRFEV6YcdcWlPULgkuD+EKIsTCtSMBFRZ0YONx9ubalx InU13P5skF01NRpvqpHJA== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeefvddrtddtgdduvdehgeekucetufdoteggodetrf dotffvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdggtfgfnhhsuhgsshgtrhhisggv pdfurfetoffkrfgpnffqhgenuceurghilhhouhhtmecufedttdenucesvcftvggtihhpih gvnhhtshculddquddttddmnecujfgurhephfffufggtgfgkfhfjgfvvefosehtjeertder tdejnecuhfhrohhmpefrrghtrhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhksh drihhmqeenucggtffrrghtthgvrhhnpeffueeiudejvdekheeuvdekfeffiedvueelteek udehjeetkeegvddugfdtgfeileenucevlhhushhtvghrufhiiigvpedvnecurfgrrhgrmh epmhgrihhlfhhrohhmpehpshesphhkshdrihhmpdhnsggprhgtphhtthhopeejpdhmohgu vgepshhmthhpohhuthdprhgtphhtthhopehgihhtshhtvghrsehpohgsohigrdgtohhmpd hrtghpthhtohepghhithesvhhgvghrrdhkvghrnhgvlhdrohhrghdprhgtphhtthhopehp vghffhesphgvfhhfrdhnvghtpdhrtghpthhtohepshgrnhgurghlshestghruhhsthihth hoohhthhhprghsthgvrdhnvghtpdhrtghpthhtohepkhgrrhhthhhikhdrudekkeesghhm rghilhdrtghomhdprhgtphhtthhopehshhgvjhhirghluhhosehgmhgrihhlrdgtohhmpd hrtghpthhtoheptghhrhhishgtohholhesthhugihfrghmihhlhidrohhrgh X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Wed, 12 Mar 2025 11:56:22 -0400 (EDT) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 6dbb1931 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Wed, 12 Mar 2025 15:56:20 +0000 (UTC) From: Patrick Steinhardt Date: Wed, 12 Mar 2025 16:56:17 +0100 Subject: [PATCH v6 11/16] refs/iterator: implement seeking for merged iterators Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Message-Id: <20250312-pks-update-ref-optimization-v6-11-f778e0414f55@pks.im> References: <20250312-pks-update-ref-optimization-v6-0-f778e0414f55@pks.im> In-Reply-To: <20250312-pks-update-ref-optimization-v6-0-f778e0414f55@pks.im> To: git@vger.kernel.org Cc: Karthik Nayak , "brian m. carlson" , Jeff King , Junio C Hamano , shejialuo , Christian Couder X-Mailer: b4 0.14.2 Implement seeking on merged iterators. The implementation is rather straight forward, with the only exception that we must not deallocate the underlying iterators once they have been exhausted. Signed-off-by: Patrick Steinhardt --- refs/iterator.c | 38 +++++++++++++++++++++++++++++--------- 1 file changed, 29 insertions(+), 9 deletions(-) diff --git a/refs/iterator.c b/refs/iterator.c index ea4db59481d..766d96e795c 100644 --- a/refs/iterator.c +++ b/refs/iterator.c @@ -96,7 +96,8 @@ int is_empty_ref_iterator(struct ref_iterator *ref_iterator) struct merge_ref_iterator { struct ref_iterator base; - struct ref_iterator *iter0, *iter1; + struct ref_iterator *iter0, *iter0_owned; + struct ref_iterator *iter1, *iter1_owned; ref_iterator_select_fn *select; void *cb_data; @@ -160,13 +161,11 @@ static int merge_ref_iterator_advance(struct ref_iterator *ref_iterator) if (!iter->current) { /* Initialize: advance both iterators to their first entries */ if ((ok = ref_iterator_advance(iter->iter0)) != ITER_OK) { - ref_iterator_free(iter->iter0); iter->iter0 = NULL; if (ok == ITER_ERROR) goto error; } if ((ok = ref_iterator_advance(iter->iter1)) != ITER_OK) { - ref_iterator_free(iter->iter1); iter->iter1 = NULL; if (ok == ITER_ERROR) goto error; @@ -177,7 +176,6 @@ static int merge_ref_iterator_advance(struct ref_iterator *ref_iterator) * entry: */ if ((ok = ref_iterator_advance(*iter->current)) != ITER_OK) { - ref_iterator_free(*iter->current); *iter->current = NULL; if (ok == ITER_ERROR) goto error; @@ -206,7 +204,6 @@ static int merge_ref_iterator_advance(struct ref_iterator *ref_iterator) if (selection & ITER_SKIP_SECONDARY) { if ((ok = ref_iterator_advance(*secondary)) != ITER_OK) { - ref_iterator_free(*secondary); *secondary = NULL; if (ok == ITER_ERROR) goto error; @@ -226,6 +223,28 @@ static int merge_ref_iterator_advance(struct ref_iterator *ref_iterator) return ITER_ERROR; } +static int merge_ref_iterator_seek(struct ref_iterator *ref_iterator, + const char *prefix) +{ + struct merge_ref_iterator *iter = + (struct merge_ref_iterator *)ref_iterator; + int ret; + + iter->current = NULL; + iter->iter0 = iter->iter0_owned; + iter->iter1 = iter->iter1_owned; + + ret = ref_iterator_seek(iter->iter0, prefix); + if (ret < 0) + return ret; + + ret = ref_iterator_seek(iter->iter1, prefix); + if (ret < 0) + return ret; + + return 0; +} + static int merge_ref_iterator_peel(struct ref_iterator *ref_iterator, struct object_id *peeled) { @@ -242,12 +261,13 @@ static void merge_ref_iterator_release(struct ref_iterator *ref_iterator) { struct merge_ref_iterator *iter = (struct merge_ref_iterator *)ref_iterator; - ref_iterator_free(iter->iter0); - ref_iterator_free(iter->iter1); + ref_iterator_free(iter->iter0_owned); + ref_iterator_free(iter->iter1_owned); } static struct ref_iterator_vtable merge_ref_iterator_vtable = { .advance = merge_ref_iterator_advance, + .seek = merge_ref_iterator_seek, .peel = merge_ref_iterator_peel, .release = merge_ref_iterator_release, }; @@ -268,8 +288,8 @@ struct ref_iterator *merge_ref_iterator_begin( */ base_ref_iterator_init(ref_iterator, &merge_ref_iterator_vtable); - iter->iter0 = iter0; - iter->iter1 = iter1; + iter->iter0 = iter->iter0_owned = iter0; + iter->iter1 = iter->iter1_owned = iter1; iter->select = select; iter->cb_data = cb_data; iter->current = NULL; From patchwork Wed Mar 12 15:56:18 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 14013667 Received: from fhigh-b4-smtp.messagingengine.com (fhigh-b4-smtp.messagingengine.com [202.12.124.155]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 6EC8F257AFE for ; Wed, 12 Mar 2025 15:56:26 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=202.12.124.155 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741794988; cv=none; b=VJ4GxZtHo7lZspiyB0iqMd79q9Nq7nHKt11cqF2fwTKxiFwhvVgweboGAStgk5Yo5bhXZaK4Vana4dslNGppyXrlJtOkpW09XloKjhD/08CGeLuFRWdK2GZwQItPfVc3d9zs3rvrwRjzOELPoI9+8B/XoDT8Yh6meo/2pFyWNVU= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741794988; c=relaxed/simple; bh=2Rs3T6Nk4x0Y4+Jzg+zwoSOY36EXw5B47DxFCxrhLl4=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:References: In-Reply-To:To:Cc; b=PtcNZlIbIAo3hLukmspjiUmr8kI0uBugY4VlyAFQQXW8c3mdjo2/ApUfTzyV57O0/pF/Wu7uIlIvr5rz2hqlRRzcvi+AghgQmxpR5sCTCChGDDa01f+H1wTb1gxWtCbrhpP4OVK+EYKu3O/SNWQjKdNZ9+ui1vwixZxjhNhmuRU= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im; spf=pass smtp.mailfrom=pks.im; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b=Sf/HqO80; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=IEKi8/qr; arc=none smtp.client-ip=202.12.124.155 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pks.im Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="Sf/HqO80"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="IEKi8/qr" Received: from phl-compute-09.internal (phl-compute-09.phl.internal [10.202.2.49]) by mailfhigh.stl.internal (Postfix) with ESMTP id 65D0E2540278; Wed, 12 Mar 2025 11:56:25 -0400 (EDT) Received: from phl-mailfrontend-01 ([10.202.2.162]) by phl-compute-09.internal (MEProxy); Wed, 12 Mar 2025 11:56:25 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :content-transfer-encoding:content-type:content-type:date:date :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to; s=fm1; t=1741794985; x=1741881385; bh=sK48vln2Hog3cXv/L4ADv8rzUxxzh7VHq1CkACqojTs=; b= Sf/HqO80AqTpo6VQNl5TSaGNdWIDeTcyFvq682KJm+6Kt052OdpYflqfWydz7M+a H0mNtbbcPmwXu9LdXHkJ5wPhe34i4w2zyu9gYnR+kBKylw8fTcf8LFMOGsu3U6vK g89atSNPAeAKvX/214iNNExf8A9Pqe/zB694C2EqcLhGmOw6+XFPlA1FNTETX/c7 QCykbxLlJaOLCuh8LJT5ecEvfHGr7AIPh4Edx2/vI1My47ewjr8VZeCvjkE2594p 0HO9fIEq6rGDOhzx+OWSk7tweCztCy3kXU1Jjiev3aGaR4fX6g+VDbdClZfDGtVw i/agWvToMMj1hyeolFlA6A== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:content-type:date:date:feedback-id:feedback-id :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to:x-me-proxy :x-me-sender:x-me-sender:x-sasl-enc; s=fm1; t=1741794985; x= 1741881385; bh=sK48vln2Hog3cXv/L4ADv8rzUxxzh7VHq1CkACqojTs=; b=I EKi8/qriCKSiiW+mZ6ztSKWJUiBuIbQo06AmODG/9PrdOlU/BOzsewtDa8vNaJWl j2mlPt9dFKXdSy44mDvQlMh+PRa6NISDyWPr7u9/1L3Xrr5ewWHZwuIESA8jRaZ5 XfxG1YDCDBMkomlfF0WSTRNWoKAjIm2Kpn0UHePZP3tp4sUXU2TbV5xY9Ljhnr2g sOeV1bnCbqE+7rMjKIS5xvpTHM9q/uOFWqNowvwC9tQseXV8ffJJ+PyUaPhbLUYs WmU76InbFRiStll0jmhCcNuiVY3DUocAhPaHajiEHIfEvwyGjRvmxWqIlIHx44y1 5K86e7HATnDUEMxsM6uHw== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeefvddrtddtgdduvdehgeelucetufdoteggodetrf dotffvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdggtfgfnhhsuhgsshgtrhhisggv pdfurfetoffkrfgpnffqhgenuceurghilhhouhhtmecufedttdenucesvcftvggtihhpih gvnhhtshculddquddttddmnecujfgurhephfffufggtgfgkfhfjgfvvefosehtjeertder tdejnecuhfhrohhmpefrrghtrhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhksh drihhmqeenucggtffrrghtthgvrhhnpeffueeiudejvdekheeuvdekfeffiedvueelteek udehjeetkeegvddugfdtgfeileenucevlhhushhtvghrufhiiigvpedvnecurfgrrhgrmh epmhgrihhlfhhrohhmpehpshesphhkshdrihhmpdhnsggprhgtphhtthhopeejpdhmohgu vgepshhmthhpohhuthdprhgtphhtthhopegthhhrihhstghoohhlsehtuhigfhgrmhhilh ihrdhorhhgpdhrtghpthhtohepshhhvghjihgrlhhuohesghhmrghilhdrtghomhdprhgt phhtthhopehkrghrthhhihhkrddukeeksehgmhgrihhlrdgtohhmpdhrtghpthhtohepgh hithhsthgvrhesphhosghogidrtghomhdprhgtphhtthhopehpvghffhesphgvfhhfrdhn vghtpdhrtghpthhtohepghhithesvhhgvghrrdhkvghrnhgvlhdrohhrghdprhgtphhtth hopehsrghnuggrlhhssegtrhhushhthihtohhothhhphgrshhtvgdrnhgvth X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Wed, 12 Mar 2025 11:56:23 -0400 (EDT) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 601addad (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Wed, 12 Mar 2025 15:56:20 +0000 (UTC) From: Patrick Steinhardt Date: Wed, 12 Mar 2025 16:56:18 +0100 Subject: [PATCH v6 12/16] refs/iterator: implement seeking for reftable iterators Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Message-Id: <20250312-pks-update-ref-optimization-v6-12-f778e0414f55@pks.im> References: <20250312-pks-update-ref-optimization-v6-0-f778e0414f55@pks.im> In-Reply-To: <20250312-pks-update-ref-optimization-v6-0-f778e0414f55@pks.im> To: git@vger.kernel.org Cc: Karthik Nayak , "brian m. carlson" , Jeff King , Junio C Hamano , shejialuo , Christian Couder X-Mailer: b4 0.14.2 Implement seeking of reftable iterators. As the low-level reftable iterators already support seeking this change is straight-forward. Two notes though: - We do not support seeking on reflog iterators. It is unclear what seeking would even look like in this context, as you typically would want to seek to a specific entry in the reflog for a specific ref. There is currently no use case for this, but if one arises in the future, we can still implement seeking at that later point. - We start to check whether `reftable_stack_init_ref_iterator()` is successful. Signed-off-by: Patrick Steinhardt --- refs/reftable-backend.c | 35 ++++++++++++++++++++++++++++++----- 1 file changed, 30 insertions(+), 5 deletions(-) diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c index 57d8512fe80..6a60b26d1b9 100644 --- a/refs/reftable-backend.c +++ b/refs/reftable-backend.c @@ -547,7 +547,7 @@ struct reftable_ref_iterator { struct reftable_ref_record ref; struct object_id oid; - const char *prefix; + char *prefix; size_t prefix_len; char **exclude_patterns; size_t exclude_patterns_index; @@ -718,6 +718,20 @@ static int reftable_ref_iterator_advance(struct ref_iterator *ref_iterator) return ITER_OK; } +static int reftable_ref_iterator_seek(struct ref_iterator *ref_iterator, + const char *prefix) +{ + struct reftable_ref_iterator *iter = + (struct reftable_ref_iterator *)ref_iterator; + + free(iter->prefix); + iter->prefix = xstrdup_or_null(prefix); + iter->prefix_len = prefix ? strlen(prefix) : 0; + iter->err = reftable_iterator_seek_ref(&iter->iter, prefix); + + return iter->err; +} + static int reftable_ref_iterator_peel(struct ref_iterator *ref_iterator, struct object_id *peeled) { @@ -744,10 +758,12 @@ static void reftable_ref_iterator_release(struct ref_iterator *ref_iterator) free(iter->exclude_patterns[i]); free(iter->exclude_patterns); } + free(iter->prefix); } static struct ref_iterator_vtable reftable_ref_iterator_vtable = { .advance = reftable_ref_iterator_advance, + .seek = reftable_ref_iterator_seek, .peel = reftable_ref_iterator_peel, .release = reftable_ref_iterator_release, }; @@ -806,8 +822,6 @@ static struct reftable_ref_iterator *ref_iterator_for_stack(struct reftable_ref_ iter = xcalloc(1, sizeof(*iter)); base_ref_iterator_init(&iter->base, &reftable_ref_iterator_vtable); - iter->prefix = prefix; - iter->prefix_len = prefix ? strlen(prefix) : 0; iter->base.oid = &iter->oid; iter->flags = flags; iter->refs = refs; @@ -821,8 +835,11 @@ static struct reftable_ref_iterator *ref_iterator_for_stack(struct reftable_ref_ if (ret) goto done; - reftable_stack_init_ref_iterator(stack, &iter->iter); - ret = reftable_iterator_seek_ref(&iter->iter, prefix); + ret = reftable_stack_init_ref_iterator(stack, &iter->iter); + if (ret) + goto done; + + ret = reftable_ref_iterator_seek(&iter->base, prefix); if (ret) goto done; @@ -2014,6 +2031,13 @@ static int reftable_reflog_iterator_advance(struct ref_iterator *ref_iterator) return ITER_OK; } +static int reftable_reflog_iterator_seek(struct ref_iterator *ref_iterator UNUSED, + const char *prefix UNUSED) +{ + BUG("reftable reflog iterator cannot be seeked"); + return -1; +} + static int reftable_reflog_iterator_peel(struct ref_iterator *ref_iterator UNUSED, struct object_id *peeled UNUSED) { @@ -2032,6 +2056,7 @@ static void reftable_reflog_iterator_release(struct ref_iterator *ref_iterator) static struct ref_iterator_vtable reftable_reflog_iterator_vtable = { .advance = reftable_reflog_iterator_advance, + .seek = reftable_reflog_iterator_seek, .peel = reftable_reflog_iterator_peel, .release = reftable_reflog_iterator_release, }; From patchwork Wed Mar 12 15:56:19 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 14013669 Received: from fout-b4-smtp.messagingengine.com (fout-b4-smtp.messagingengine.com [202.12.124.147]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 9F5E42580F0 for ; Wed, 12 Mar 2025 15:56:26 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=202.12.124.147 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741794988; cv=none; b=ay7ijbKmbstrQ3xMUawv+iak3orqDux0k6urd0FPZgFboMw4BQBNx8JW30NEumDIsDZaKeNF6OlZcsvpHca/jnQF6QmWmyIx/cU67+G/bwLgOqizpGDnqm7IEV0PaHgETwrS8RXq83L3UPRnuwUJMYg/hIDV5UXbNQ5cx6qpJeA= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741794988; c=relaxed/simple; bh=bXASNitFs176Fox6XS/61l5fh2zndLjWHrIzTnkK8to=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:References: In-Reply-To:To:Cc; b=BNXBii/s9OiexPAJQ0BTHhkr1leBjtpxmtx9+LEFkZ1L9DTQAKMY3mgvamGWiymrSq0EYfC/fYJmnpk0JlpbeK8PQG0ulbs/Bbkl684uRcsoFCVR0uZdbmYVX1mIMLixW8+HWMPf+acKrtpx63z6CNucyjZ1X9ePrF5ynuCcPuM= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im; spf=pass smtp.mailfrom=pks.im; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b=X0zkKVMT; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=KedLbHMa; arc=none smtp.client-ip=202.12.124.147 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pks.im Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="X0zkKVMT"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="KedLbHMa" Received: from phl-compute-13.internal (phl-compute-13.phl.internal [10.202.2.53]) by mailfout.stl.internal (Postfix) with ESMTP id 8A7081140213; Wed, 12 Mar 2025 11:56:25 -0400 (EDT) Received: from phl-mailfrontend-02 ([10.202.2.163]) by phl-compute-13.internal (MEProxy); Wed, 12 Mar 2025 11:56:25 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :content-transfer-encoding:content-type:content-type:date:date :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to; s=fm1; t=1741794985; x=1741881385; bh=7zDBP21lJvED/SD8pZF73JifowXugfFlCjgTwRqI+W4=; b= X0zkKVMTiPbPnznmjsbxvLsflDC5OMgSK+lVG05FfEe6hUCeVSRIKQ/fke5ch1iF 7vUd+qe9exbcgWoJegAGZ/qzhxVyzjhb8mXo0ZwpkI/pRTdew3kG6TEjV4nNVpRV Rh/ht6Kdczb1EmbS8OG25m6TR8aXTdKlNONRMzc1lLeEohh4OCPXT+wAEhbgxXLm 1dcr0dJBGLAOgNFikb7O1553uJNNFSbucvOpeF2NNp4GXqNmjfbsXZi8hMsKTj8W TiuS/pYMu6Kz1NWGph/fx17CXYQqrUYPtwwQozt/mfr9XpmEp0BFbEF37LbYlkrr 47WGWo09G9ElcDrncERAaQ== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:content-type:date:date:feedback-id:feedback-id :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to:x-me-proxy :x-me-sender:x-me-sender:x-sasl-enc; s=fm1; t=1741794985; x= 1741881385; bh=7zDBP21lJvED/SD8pZF73JifowXugfFlCjgTwRqI+W4=; b=K edLbHMa+bF9PaFbcrX30bJpZe3n1K/XZGlQ7ADOZVn1HplsgSjKbozckhi82DQNJ 1e+bcahNCfL3dnuSzm17gmA7UIYl+SS47zq0c5/YirY0RH/TI4zKVrXEm8XJ2W4c j6zJg72DCJM+d1GPw4dDxox7Z2nAFBmIzFcS+mUX5/dZXOPkZOTC4ZsGKQNOE4Pw Xav5LT8f+cKhOcQCeWVWFQ64hDU0UXflYhMUqKjpQXms60uiGUeSOL/y835T5SSx EI0rcoQfCX6tl9uUgTUxq1h5RgCHfqilYIocESomOCHHU9jOe+kpx8BylfgOw8pl e4lAcZaHWTK17YEzDW7JA== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeefvddrtddtgdduvdehgeekucetufdoteggodetrf dotffvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdggtfgfnhhsuhgsshgtrhhisggv pdfurfetoffkrfgpnffqhgenuceurghilhhouhhtmecufedttdenucesvcftvggtihhpih gvnhhtshculddquddttddmnecujfgurhephfffufggtgfgkfhfjgfvvefosehtjeertder tdejnecuhfhrohhmpefrrghtrhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhksh drihhmqeenucggtffrrghtthgvrhhnpeffueeiudejvdekheeuvdekfeffiedvueelteek udehjeetkeegvddugfdtgfeileenucevlhhushhtvghrufhiiigvpedvnecurfgrrhgrmh epmhgrihhlfhhrohhmpehpshesphhkshdrihhmpdhnsggprhgtphhtthhopeejpdhmohgu vgepshhmthhpohhuthdprhgtphhtthhopegthhhrihhstghoohhlsehtuhigfhgrmhhilh ihrdhorhhgpdhrtghpthhtohepghhithesvhhgvghrrdhkvghrnhgvlhdrohhrghdprhgt phhtthhopehshhgvjhhirghluhhosehgmhgrihhlrdgtohhmpdhrtghpthhtohepshgrnh gurghlshestghruhhsthihthhoohhthhhprghsthgvrdhnvghtpdhrtghpthhtohepphgv fhhfsehpvghffhdrnhgvthdprhgtphhtthhopehkrghrthhhihhkrddukeeksehgmhgrih hlrdgtohhmpdhrtghpthhtohepghhithhsthgvrhesphhosghogidrtghomh X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Wed, 12 Mar 2025 11:56:24 -0400 (EDT) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 2ef55a39 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Wed, 12 Mar 2025 15:56:21 +0000 (UTC) From: Patrick Steinhardt Date: Wed, 12 Mar 2025 16:56:19 +0100 Subject: [PATCH v6 13/16] refs/iterator: implement seeking for ref-cache iterators Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Message-Id: <20250312-pks-update-ref-optimization-v6-13-f778e0414f55@pks.im> References: <20250312-pks-update-ref-optimization-v6-0-f778e0414f55@pks.im> In-Reply-To: <20250312-pks-update-ref-optimization-v6-0-f778e0414f55@pks.im> To: git@vger.kernel.org Cc: Karthik Nayak , "brian m. carlson" , Jeff King , Junio C Hamano , shejialuo , Christian Couder X-Mailer: b4 0.14.2 Implement seeking of ref-cache iterators. This is done by splitting most of the logic to seek iterators out of `cache_ref_iterator_begin()` and putting it into `cache_ref_iterator_seek()` so that we can reuse the logic. Note that we cannot use the optimization anymore where we return an empty ref iterator when there aren't any references, as otherwise it wouldn't be possible to reseek the iterator to a different prefix that may exist. This shouldn't be much of a performance concern though as we now start to bail out early in case `advance()` sees that there are no more directories to be searched. Signed-off-by: Patrick Steinhardt --- refs/ref-cache.c | 79 ++++++++++++++++++++++++++++++++++++-------------------- 1 file changed, 51 insertions(+), 28 deletions(-) diff --git a/refs/ref-cache.c b/refs/ref-cache.c index 6457e02c1ea..c1f1bab1d50 100644 --- a/refs/ref-cache.c +++ b/refs/ref-cache.c @@ -362,9 +362,7 @@ struct cache_ref_iterator { struct ref_iterator base; /* - * The number of levels currently on the stack. This is always - * at least 1, because when it becomes zero the iteration is - * ended and this struct is freed. + * The number of levels currently on the stack. */ size_t levels_nr; @@ -376,7 +374,7 @@ struct cache_ref_iterator { * The prefix is matched textually, without regard for path * component boundaries. */ - const char *prefix; + char *prefix; /* * A stack of levels. levels[0] is the uppermost level that is @@ -389,6 +387,9 @@ struct cache_ref_iterator { struct cache_ref_iterator_level *levels; struct repository *repo; + struct ref_cache *cache; + + int prime_dir; }; static int cache_ref_iterator_advance(struct ref_iterator *ref_iterator) @@ -396,6 +397,9 @@ static int cache_ref_iterator_advance(struct ref_iterator *ref_iterator) struct cache_ref_iterator *iter = (struct cache_ref_iterator *)ref_iterator; + if (!iter->levels_nr) + return ITER_DONE; + while (1) { struct cache_ref_iterator_level *level = &iter->levels[iter->levels_nr - 1]; @@ -444,6 +448,41 @@ static int cache_ref_iterator_advance(struct ref_iterator *ref_iterator) } } +static int cache_ref_iterator_seek(struct ref_iterator *ref_iterator, + const char *prefix) +{ + struct cache_ref_iterator *iter = + (struct cache_ref_iterator *)ref_iterator; + struct cache_ref_iterator_level *level; + struct ref_dir *dir; + + dir = get_ref_dir(iter->cache->root); + if (prefix && *prefix) + dir = find_containing_dir(dir, prefix); + if (!dir) { + iter->levels_nr = 0; + return 0; + } + + if (iter->prime_dir) + prime_ref_dir(dir, prefix); + iter->levels_nr = 1; + level = &iter->levels[0]; + level->index = -1; + level->dir = dir; + + if (prefix && *prefix) { + free(iter->prefix); + iter->prefix = xstrdup(prefix); + level->prefix_state = PREFIX_WITHIN_DIR; + } else { + FREE_AND_NULL(iter->prefix); + level->prefix_state = PREFIX_CONTAINS_DIR; + } + + return 0; +} + static int cache_ref_iterator_peel(struct ref_iterator *ref_iterator, struct object_id *peeled) { @@ -456,12 +495,13 @@ static void cache_ref_iterator_release(struct ref_iterator *ref_iterator) { struct cache_ref_iterator *iter = (struct cache_ref_iterator *)ref_iterator; - free((char *)iter->prefix); + free(iter->prefix); free(iter->levels); } static struct ref_iterator_vtable cache_ref_iterator_vtable = { .advance = cache_ref_iterator_advance, + .seek = cache_ref_iterator_seek, .peel = cache_ref_iterator_peel, .release = cache_ref_iterator_release, }; @@ -471,39 +511,22 @@ struct ref_iterator *cache_ref_iterator_begin(struct ref_cache *cache, struct repository *repo, int prime_dir) { - struct ref_dir *dir; struct cache_ref_iterator *iter; struct ref_iterator *ref_iterator; - struct cache_ref_iterator_level *level; - - dir = get_ref_dir(cache->root); - if (prefix && *prefix) - dir = find_containing_dir(dir, prefix); - if (!dir) - /* There's nothing to iterate over. */ - return empty_ref_iterator_begin(); - - if (prime_dir) - prime_ref_dir(dir, prefix); CALLOC_ARRAY(iter, 1); ref_iterator = &iter->base; base_ref_iterator_init(ref_iterator, &cache_ref_iterator_vtable); ALLOC_GROW(iter->levels, 10, iter->levels_alloc); - iter->levels_nr = 1; - level = &iter->levels[0]; - level->index = -1; - level->dir = dir; + iter->repo = repo; + iter->cache = cache; + iter->prime_dir = prime_dir; - if (prefix && *prefix) { - iter->prefix = xstrdup(prefix); - level->prefix_state = PREFIX_WITHIN_DIR; - } else { - level->prefix_state = PREFIX_CONTAINS_DIR; + if (cache_ref_iterator_seek(&iter->base, prefix) < 0) { + ref_iterator_free(&iter->base); + return NULL; } - iter->repo = repo; - return ref_iterator; } From patchwork Wed Mar 12 15:56:20 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 14013668 Received: from fhigh-b4-smtp.messagingengine.com (fhigh-b4-smtp.messagingengine.com [202.12.124.155]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id AA0C6258CDB for ; Wed, 12 Mar 2025 15:56:26 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=202.12.124.155 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741794988; cv=none; b=DtiJb2tAiJaoRx+98EwGDwkT5DHOxlulPyk912Z3S1ZU3MsL/q5msys+AmBeOY9FJ617V2Fa7c6Du3w0eygCyppa3wj5wV9LHzAM0fgfwJuFrzZ8cy65Hm6u7TNjuYLyTidoae4xz/8e0XSCaDC3AIX+RKjx1ZDzWMG86PMdSCE= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741794988; c=relaxed/simple; bh=jWJmG5b6aaqEzGjDMnZ6Z0GkMINs1oUh8NwnYkRCEs4=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:References: In-Reply-To:To:Cc; b=f6Vo2MibGYmMFwN1XVLl8CRJX7F4ruIi9Rjy/TJ0TCFrupOrvbFVWvwjUhpyLPZptT11QmXchb+ODunMqD4ESzUYORw8NxoceQ4k/Ei1WVUKJ1zdqEDF8HVJPtnBHUSNW7YkaI9KkhoZ0sU2TbTnUsMYF/KpjSkXXZj27fUcii8= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im; spf=pass smtp.mailfrom=pks.im; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b=XWa3oJC2; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=0DikXhoF; arc=none smtp.client-ip=202.12.124.155 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pks.im Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="XWa3oJC2"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="0DikXhoF" Received: from phl-compute-12.internal (phl-compute-12.phl.internal [10.202.2.52]) by mailfhigh.stl.internal (Postfix) with ESMTP id A4F202540312; Wed, 12 Mar 2025 11:56:25 -0400 (EDT) Received: from phl-mailfrontend-02 ([10.202.2.163]) by phl-compute-12.internal (MEProxy); Wed, 12 Mar 2025 11:56:25 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :content-transfer-encoding:content-type:content-type:date:date :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to; s=fm1; t=1741794985; x=1741881385; bh=uXhF9ndDLYF46pzv1SWMMdCFFhDN44mKIIgkHIu0E40=; b= XWa3oJC2qGptQNKMJd4NKVR/zgIXKMGG+yOoUTuO8KkXl3cO1AS00qjSADciOUia Mz7K0ze214KM+9H+ZwOHgUGdUwhkT/TBN6UWmRnKQMUbqPWbNFSZXdg46B3C4eBT 6bzk83Q2NNIENXAvRMIuyhYaLF1MuhdW7RIuljtDcmNNXyBqRFkL7sWvg5+EpKA7 gUKzToK2jcXuPtgTbKmnYCT4bjB+yPFEmCqZJR8c5ZwIRYcTbiXh86Kpg7NL3KuN Y9EJb4qxwVPrNnEnIhchA+WHX/+HWPMBuz4UCEbeI48SBlGupDi7e2VaVASXHQ1y UMBu7xHjPR7R2/s5zFME/A== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:content-type:date:date:feedback-id:feedback-id :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to:x-me-proxy :x-me-sender:x-me-sender:x-sasl-enc; s=fm1; t=1741794985; x= 1741881385; bh=uXhF9ndDLYF46pzv1SWMMdCFFhDN44mKIIgkHIu0E40=; b=0 DikXhoFA1sdwB3oArOerATljT6wPAossvTcglDnaRvk5B9GDdySm4lLWBXdqtqn1 /qx7SUQyg6RCo3UivuaC5VjwlmuBsB2OBrHf2/Fc3NBS6E0OFcHIK6MKj/rN6AQU OfVUkt4qahJ7QaBqh0hrJNUB2fUCkGHZ8Ob1uIiBk7M5KBfcmF341RnXMhQ+H94X xCP+8zYK2YKtGyvYKMCbTKHfeoRy56c2f+JIMBAO4m58FYa3s90j35ADKne+wGW1 tfFVtX9dWqMz9pmyIIWMQIFz5RWG0I+Xi3Ps1T6tChMSxTVS2mkeNuiwUI3YG2QB JRib0pgCd/UTUQZlGQTmA== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeefvddrtddtgdduvdehgeekucetufdoteggodetrf dotffvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdggtfgfnhhsuhgsshgtrhhisggv pdfurfetoffkrfgpnffqhgenuceurghilhhouhhtmecufedttdenucesvcftvggtihhpih gvnhhtshculddquddttddmnecujfgurhephfffufggtgfgkfhfjgfvvefosehtjeertder tdejnecuhfhrohhmpefrrghtrhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhksh drihhmqeenucggtffrrghtthgvrhhnpeffueeiudejvdekheeuvdekfeffiedvueelteek udehjeetkeegvddugfdtgfeileenucevlhhushhtvghrufhiiigvpeefnecurfgrrhgrmh epmhgrihhlfhhrohhmpehpshesphhkshdrihhmpdhnsggprhgtphhtthhopeejpdhmohgu vgepshhmthhpohhuthdprhgtphhtthhopehsrghnuggrlhhssegtrhhushhthihtohhoth hhphgrshhtvgdrnhgvthdprhgtphhtthhopehkrghrthhhihhkrddukeeksehgmhgrihhl rdgtohhmpdhrtghpthhtohepghhithesvhhgvghrrdhkvghrnhgvlhdrohhrghdprhgtph htthhopehshhgvjhhirghluhhosehgmhgrihhlrdgtohhmpdhrtghpthhtohepphgvfhhf sehpvghffhdrnhgvthdprhgtphhtthhopegthhhrihhstghoohhlsehtuhigfhgrmhhilh ihrdhorhhgpdhrtghpthhtohepghhithhsthgvrhesphhosghogidrtghomh X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Wed, 12 Mar 2025 11:56:24 -0400 (EDT) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 152a7844 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Wed, 12 Mar 2025 15:56:22 +0000 (UTC) From: Patrick Steinhardt Date: Wed, 12 Mar 2025 16:56:20 +0100 Subject: [PATCH v6 14/16] refs/iterator: implement seeking for packed-ref iterators Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Message-Id: <20250312-pks-update-ref-optimization-v6-14-f778e0414f55@pks.im> References: <20250312-pks-update-ref-optimization-v6-0-f778e0414f55@pks.im> In-Reply-To: <20250312-pks-update-ref-optimization-v6-0-f778e0414f55@pks.im> To: git@vger.kernel.org Cc: Karthik Nayak , "brian m. carlson" , Jeff King , Junio C Hamano , shejialuo , Christian Couder X-Mailer: b4 0.14.2 Implement seeking of `packed-ref` iterators. The implementation is again straight forward, except that we cannot continue to use the prefix iterator as we would otherwise not be able to reseek the iterator anymore in case one first asks for an empty and then for a non-empty prefix. Instead, we open-code the logic to in `advance()`. Signed-off-by: Patrick Steinhardt --- refs/packed-backend.c | 65 ++++++++++++++++++++++++++++++++++----------------- 1 file changed, 43 insertions(+), 22 deletions(-) diff --git a/refs/packed-backend.c b/refs/packed-backend.c index 38a1956d1a8..f4c82ba2c7d 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -819,6 +819,8 @@ struct packed_ref_iterator { struct snapshot *snapshot; + char *prefix; + /* The current position in the snapshot's buffer: */ const char *pos; @@ -841,11 +843,9 @@ struct packed_ref_iterator { }; /* - * Move the iterator to the next record in the snapshot, without - * respect for whether the record is actually required by the current - * iteration. Adjust the fields in `iter` and return `ITER_OK` or - * `ITER_DONE`. This function does not free the iterator in the case - * of `ITER_DONE`. + * Move the iterator to the next record in the snapshot. Adjust the fields in + * `iter` and return `ITER_OK` or `ITER_DONE`. This function does not free the + * iterator in the case of `ITER_DONE`. */ static int next_record(struct packed_ref_iterator *iter) { @@ -942,6 +942,9 @@ static int packed_ref_iterator_advance(struct ref_iterator *ref_iterator) int ok; while ((ok = next_record(iter)) == ITER_OK) { + const char *refname = iter->base.refname; + const char *prefix = iter->prefix; + if (iter->flags & DO_FOR_EACH_PER_WORKTREE_ONLY && !is_per_worktree_ref(iter->base.refname)) continue; @@ -951,12 +954,41 @@ static int packed_ref_iterator_advance(struct ref_iterator *ref_iterator) &iter->oid, iter->flags)) continue; + while (prefix && *prefix) { + if (*refname < *prefix) + BUG("packed-refs backend yielded reference preceding its prefix"); + else if (*refname > *prefix) + return ITER_DONE; + prefix++; + refname++; + } + return ITER_OK; } return ok; } +static int packed_ref_iterator_seek(struct ref_iterator *ref_iterator, + const char *prefix) +{ + struct packed_ref_iterator *iter = + (struct packed_ref_iterator *)ref_iterator; + const char *start; + + if (prefix && *prefix) + start = find_reference_location(iter->snapshot, prefix, 0); + else + start = iter->snapshot->start; + + free(iter->prefix); + iter->prefix = xstrdup_or_null(prefix); + iter->pos = start; + iter->eof = iter->snapshot->eof; + + return 0; +} + static int packed_ref_iterator_peel(struct ref_iterator *ref_iterator, struct object_id *peeled) { @@ -979,11 +1011,13 @@ static void packed_ref_iterator_release(struct ref_iterator *ref_iterator) (struct packed_ref_iterator *)ref_iterator; strbuf_release(&iter->refname_buf); free(iter->jump); + free(iter->prefix); release_snapshot(iter->snapshot); } static struct ref_iterator_vtable packed_ref_iterator_vtable = { .advance = packed_ref_iterator_advance, + .seek = packed_ref_iterator_seek, .peel = packed_ref_iterator_peel, .release = packed_ref_iterator_release, }; @@ -1097,7 +1131,6 @@ static struct ref_iterator *packed_ref_iterator_begin( { struct packed_ref_store *refs; struct snapshot *snapshot; - const char *start; struct packed_ref_iterator *iter; struct ref_iterator *ref_iterator; unsigned int required_flags = REF_STORE_READ; @@ -1113,14 +1146,6 @@ static struct ref_iterator *packed_ref_iterator_begin( */ snapshot = get_snapshot(refs); - if (prefix && *prefix) - start = find_reference_location(snapshot, prefix, 0); - else - start = snapshot->start; - - if (start == snapshot->eof) - return empty_ref_iterator_begin(); - CALLOC_ARRAY(iter, 1); ref_iterator = &iter->base; base_ref_iterator_init(ref_iterator, &packed_ref_iterator_vtable); @@ -1130,19 +1155,15 @@ static struct ref_iterator *packed_ref_iterator_begin( iter->snapshot = snapshot; acquire_snapshot(snapshot); - - iter->pos = start; - iter->eof = snapshot->eof; strbuf_init(&iter->refname_buf, 0); - iter->base.oid = &iter->oid; - iter->repo = ref_store->repo; iter->flags = flags; - if (prefix && *prefix) - /* Stop iteration after we've gone *past* prefix: */ - ref_iterator = prefix_ref_iterator_begin(ref_iterator, prefix, 0); + if (packed_ref_iterator_seek(&iter->base, prefix) < 0) { + ref_iterator_free(&iter->base); + return NULL; + } return ref_iterator; } From patchwork Wed Mar 12 15:56:21 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 14013670 Received: from fhigh-b4-smtp.messagingengine.com (fhigh-b4-smtp.messagingengine.com [202.12.124.155]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 413F0259C83 for ; Wed, 12 Mar 2025 15:56:28 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=202.12.124.155 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741794989; cv=none; b=QeOZ0vQMzq6BrWXUgQ0oUsG4jH42kb3jzLEWAJpax3idFVcR8wS3oMNdOB3EvDm87RRP9U65HQ8OqdzNa6ldntXZQzjBMHUMmoS1CjIiw/PvML9vpXvbNCsORm+ABljPCcwkbqcqZXFRzUeh1ITOpwMIZm8lKnRplxHd2e+RejY= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741794989; c=relaxed/simple; bh=i9zsBYfFR4LpMFSxhoX6blqilRvlU2We+avpj2EMjqs=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:References: In-Reply-To:To:Cc; b=XT601PN9wvn2j52VCpXu3VjixD7gk6L+4JeypsgsyqV8Tk4ZtuI0HuuHoFK+VSpbxiKkc4IVfzH2V5SEiDpp4GIJHVafZGt4Wim+AMyGzAZx/hxlWAU5+HdjvdZFAEESz8plQdoVQYI6VOVeeAZdkORUZRayM4kWpdPMH0ym2iI= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im; spf=pass smtp.mailfrom=pks.im; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b=jYstaYM/; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=EHTfvNAo; arc=none smtp.client-ip=202.12.124.155 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pks.im Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="jYstaYM/"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="EHTfvNAo" Received: from phl-compute-08.internal (phl-compute-08.phl.internal [10.202.2.48]) by mailfhigh.stl.internal (Postfix) with ESMTP id 4C058254030A; Wed, 12 Mar 2025 11:56:27 -0400 (EDT) Received: from phl-mailfrontend-01 ([10.202.2.162]) by phl-compute-08.internal (MEProxy); Wed, 12 Mar 2025 11:56:27 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :content-transfer-encoding:content-type:content-type:date:date :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to; s=fm1; t=1741794987; x=1741881387; bh=G96edxJMxXRHToB7xNoWWK9UN+aH2smKObNcv11cvyk=; b= jYstaYM/p03TJVC+OzOfE787+R+mtjK1eAtKZLepmQ3F18Ue/QfNh5nSdKSBz52s SKegaw9Fume1w4ectBtHhq4XCo6k9F44u0xNDTumRvYEFLtgaL5pzE+2SVhFjfEe CM3LfVT/IgvflBlJNk7Q1Mw1clr0ToIpmMQseUBfQyWOHt1feZ9sx+ardPQWl1uK o38kbIKz5tLMoym1xtLGb+IaO3vGWGBEwkiMz4yJ36UHVLLhWdizG1GgPbu4JlUV W9yq703CbqgE7TPHqlsgUBeZPMFirMYjwG1dSAIWRCzSMA3mzG+itSYBDnRRZ7bX 1ToyWa0A+ZMyvHh/DdEt0A== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:content-type:date:date:feedback-id:feedback-id :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to:x-me-proxy :x-me-sender:x-me-sender:x-sasl-enc; s=fm1; t=1741794987; x= 1741881387; bh=G96edxJMxXRHToB7xNoWWK9UN+aH2smKObNcv11cvyk=; b=E HTfvNAoXCqTOtiJm5fJ/u4HBK6xxq0G6b1zCdym0j/+gYZti8lvWobF6TQV3rrHP 2a8gCpc2SNIssXXV170XKcdTML730l6OI0pF8hCHG9qVDrDVj8pDmZiAcrwNTQL5 vSiE1bjRT2Tp5nz+HlKeOqkQIcuZh02LajdnqAc08ALWACHzx0lzs8WpcleXDbg8 aUV66Hxctb+/nSuUO2QOxFwOPXojOJqWsWjboHcdvF/tQXP3pbrd1pmn3BdxLM1+ x/Z0v28xbYZTRjRFjbjluSmHSdbNOWLig0olskkeD7f/t8t7tNLPxRwIs7MZSjVe /hcSSRzmANRPFzLtIwVhg== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeefvddrtddtgdduvdehgeekucetufdoteggodetrf dotffvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdggtfgfnhhsuhgsshgtrhhisggv pdfurfetoffkrfgpnffqhgenuceurghilhhouhhtmecufedttdenucesvcftvggtihhpih gvnhhtshculddquddttddmnecujfgurhephfffufggtgfgkfhfjgfvvefosehtjeertder tdejnecuhfhrohhmpefrrghtrhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhksh drihhmqeenucggtffrrghtthgvrhhnpeffueeiudejvdekheeuvdekfeffiedvueelteek udehjeetkeegvddugfdtgfeileenucevlhhushhtvghrufhiiigvpedunecurfgrrhgrmh epmhgrihhlfhhrohhmpehpshesphhkshdrihhmpdhnsggprhgtphhtthhopeejpdhmohgu vgepshhmthhpohhuthdprhgtphhtthhopehpvghffhesphgvfhhfrdhnvghtpdhrtghpth htohepghhithesvhhgvghrrdhkvghrnhgvlhdrohhrghdprhgtphhtthhopehgihhtshht vghrsehpohgsohigrdgtohhmpdhrtghpthhtohepshgrnhgurghlshestghruhhsthihth hoohhthhhprghsthgvrdhnvghtpdhrtghpthhtohepshhhvghjihgrlhhuohesghhmrghi lhdrtghomhdprhgtphhtthhopehkrghrthhhihhkrddukeeksehgmhgrihhlrdgtohhmpd hrtghpthhtoheptghhrhhishgtohholhesthhugihfrghmihhlhidrohhrgh X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Wed, 12 Mar 2025 11:56:25 -0400 (EDT) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 49bb309d (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Wed, 12 Mar 2025 15:56:23 +0000 (UTC) From: Patrick Steinhardt Date: Wed, 12 Mar 2025 16:56:21 +0100 Subject: [PATCH v6 15/16] refs/iterator: implement seeking for files iterators Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Message-Id: <20250312-pks-update-ref-optimization-v6-15-f778e0414f55@pks.im> References: <20250312-pks-update-ref-optimization-v6-0-f778e0414f55@pks.im> In-Reply-To: <20250312-pks-update-ref-optimization-v6-0-f778e0414f55@pks.im> To: git@vger.kernel.org Cc: Karthik Nayak , "brian m. carlson" , Jeff King , Junio C Hamano , shejialuo , Christian Couder X-Mailer: b4 0.14.2 Implement seeking for "files" iterators. As we simply use a ref-cache iterator under the hood the implementation is straight-forward. Note that we do not implement seeking on reflog iterators, same as with the "reftable" backend. Signed-off-by: Patrick Steinhardt --- refs/files-backend.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/refs/files-backend.c b/refs/files-backend.c index e97a267ad65..5f921e85eb4 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -918,6 +918,14 @@ static int files_ref_iterator_advance(struct ref_iterator *ref_iterator) return ok; } +static int files_ref_iterator_seek(struct ref_iterator *ref_iterator, + const char *prefix) +{ + struct files_ref_iterator *iter = + (struct files_ref_iterator *)ref_iterator; + return ref_iterator_seek(iter->iter0, prefix); +} + static int files_ref_iterator_peel(struct ref_iterator *ref_iterator, struct object_id *peeled) { @@ -936,6 +944,7 @@ static void files_ref_iterator_release(struct ref_iterator *ref_iterator) static struct ref_iterator_vtable files_ref_iterator_vtable = { .advance = files_ref_iterator_advance, + .seek = files_ref_iterator_seek, .peel = files_ref_iterator_peel, .release = files_ref_iterator_release, }; @@ -2294,6 +2303,12 @@ static int files_reflog_iterator_advance(struct ref_iterator *ref_iterator) return ok; } +static int files_reflog_iterator_seek(struct ref_iterator *ref_iterator UNUSED, + const char *prefix UNUSED) +{ + BUG("ref_iterator_seek() called for reflog_iterator"); +} + static int files_reflog_iterator_peel(struct ref_iterator *ref_iterator UNUSED, struct object_id *peeled UNUSED) { @@ -2309,6 +2324,7 @@ static void files_reflog_iterator_release(struct ref_iterator *ref_iterator) static struct ref_iterator_vtable files_reflog_iterator_vtable = { .advance = files_reflog_iterator_advance, + .seek = files_reflog_iterator_seek, .peel = files_reflog_iterator_peel, .release = files_reflog_iterator_release, }; From patchwork Wed Mar 12 15:56:22 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 14013671 Received: from fhigh-b4-smtp.messagingengine.com (fhigh-b4-smtp.messagingengine.com [202.12.124.155]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 7327F258CC2 for ; Wed, 12 Mar 2025 15:56:28 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=202.12.124.155 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741794990; cv=none; b=ZaeOLXO5N5dElT1ulC2MrIp5aK2q0TUdGstIiTzyXbIr42e4djTmHaXtKOul83KyLZU8itH0TJW7eHf3Uhjby+VqcFr9+CFWSHtRhvX254qDB6ObTAPKwBrzjaM5r5joCnALpQvGp+qYLuDvfC22h1Iq8e5A+yj1ZQsJW8vj6Ec= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741794990; c=relaxed/simple; bh=5tg+G2Clnt85TH+wt7ymKR0AYZTAOvK1o6buBp2mvCs=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:References: In-Reply-To:To:Cc; b=jGNRelKdLOS4A3a0BPRobd4HIqQ8WOoWcdVoKkJs/PGVS3jwfzyeemMc555P3YGtZVnZEU6qnQqNnsLuaEjXKRCDuyTa+skgUvBbVS+t385Z7GNhw9LbvW/92z3xy38droJi/Bk6sE7fFh/nkv6DiVmrUAv4pxBcSwypvY3fHwE= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im; spf=pass smtp.mailfrom=pks.im; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b=h1LmyMzy; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=wURCI9t6; arc=none smtp.client-ip=202.12.124.155 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pks.im Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="h1LmyMzy"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="wURCI9t6" Received: from phl-compute-08.internal (phl-compute-08.phl.internal [10.202.2.48]) by mailfhigh.stl.internal (Postfix) with ESMTP id 81F462540313; Wed, 12 Mar 2025 11:56:27 -0400 (EDT) Received: from phl-mailfrontend-02 ([10.202.2.163]) by phl-compute-08.internal (MEProxy); Wed, 12 Mar 2025 11:56:27 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :content-transfer-encoding:content-type:content-type:date:date :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to; s=fm1; t=1741794987; x=1741881387; bh=IIKO4UwfFdArCIOPXfyULgZLN5Z5tkYDFn8j16KhbNk=; b= h1LmyMzy7v6qjksY1xhuILf1GB/IlKlryCdrvV/w41xysu61r5xyvaRYK7OxXTS4 hZYOq/qPclmANa+rFGJ4Z7of9SMKoxAjdTd5/m5ZoFtYxMGTAGXnhpQETKCdHhVq 7rEpFW3IM/q+aem1MrUvmq8WWNz1USb6/apf+QzTvYIjKpPRO4mOxiO26UhH7/V0 jWapKr67BmcZjRLE57GlS6y7Shrj+sLZYFPICDR/GvCM0QpdXBuMhWCQtXP1kzSB 1JPQzWzubGgvdFI6mCKV1Mc+iltkY+ejV6D8Qg5eKBbgvo7XBUAJNY1I+TP5IuUa /BeVWowDgrzs50ot0qVbRA== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:content-type:date:date:feedback-id:feedback-id :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to:x-me-proxy :x-me-sender:x-me-sender:x-sasl-enc; s=fm1; t=1741794987; x= 1741881387; bh=IIKO4UwfFdArCIOPXfyULgZLN5Z5tkYDFn8j16KhbNk=; b=w URCI9t6OXYojvoKRZI7vQFqPwS0qdhUJtrO7CKc2Vgyl5nxE1oU5iPPBW94PJfeO m+KjHax1QchvmMZwyBX4iC7VWiw50ozYRg//CjRyHLcxeceLg2asBII5sFJe8b4H Fa3ioSueY1UjkERiMCVvwIXnRwjZal7ivKIFrcz+cttV3hl+HqGTO4aD1z+Gw74z x5fVJRPpaVuydA7E/cXdWDduPsPp24rYCE3gv3P4FiYZizzhpgBL5Ig753+SV+k2 YfRtJxmh2Z0xRPHmEgmNrShAh//dv81mvkLrKof/payfwv0F/drS3C3X5ydPMvx/ 0SMC2/oj6RxhexNcnA8Bw== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeefvddrtddtgdduvdehgeekucetufdoteggodetrf dotffvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdggtfgfnhhsuhgsshgtrhhisggv pdfurfetoffkrfgpnffqhgenuceurghilhhouhhtmecufedttdenucesvcftvggtihhpih gvnhhtshculddquddttddmnecujfgurhephfffufggtgfgkfhfjgfvvefosehtkeertder tdejnecuhfhrohhmpefrrghtrhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhksh drihhmqeenucggtffrrghtthgvrhhnpeefhfeugeelheefjeektdffhedvhfdvteefgfdt udffudevveetgeeuuedtkefhgeenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmh epmhgrihhlfhhrohhmpehpshesphhkshdrihhmpdhnsggprhgtphhtthhopeejpdhmohgu vgepshhmthhpohhuthdprhgtphhtthhopehgihhtshhtvghrsehpohgsohigrdgtohhmpd hrtghpthhtohepkhgrrhhthhhikhdrudekkeesghhmrghilhdrtghomhdprhgtphhtthho pehshhgvjhhirghluhhosehgmhgrihhlrdgtohhmpdhrtghpthhtohepshgrnhgurghlsh estghruhhsthihthhoohhthhhprghsthgvrdhnvghtpdhrtghpthhtoheptghhrhhishgt ohholhesthhugihfrghmihhlhidrohhrghdprhgtphhtthhopehpvghffhesphgvfhhfrd hnvghtpdhrtghpthhtohepghhithesvhhgvghrrdhkvghrnhgvlhdrohhrgh X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Wed, 12 Mar 2025 11:56:25 -0400 (EDT) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 298ff104 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Wed, 12 Mar 2025 15:56:24 +0000 (UTC) From: Patrick Steinhardt Date: Wed, 12 Mar 2025 16:56:22 +0100 Subject: [PATCH v6 16/16] refs: reuse iterators when determining refname availability Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Message-Id: <20250312-pks-update-ref-optimization-v6-16-f778e0414f55@pks.im> References: <20250312-pks-update-ref-optimization-v6-0-f778e0414f55@pks.im> In-Reply-To: <20250312-pks-update-ref-optimization-v6-0-f778e0414f55@pks.im> To: git@vger.kernel.org Cc: Karthik Nayak , "brian m. carlson" , Jeff King , Junio C Hamano , shejialuo , Christian Couder X-Mailer: b4 0.14.2 When verifying whether refnames are available we have to verify whether any reference exists that is nested under the current reference. E.g. given a reference "refs/heads/foo", we must make sure that there is no other reference "refs/heads/foo/*". This check is performed using a ref iterator with the prefix set to the nested reference namespace. Until now it used to not be possible to reseek iterators, so we always had to reallocate the iterator for every single reference we're about to check. This keeps us from reusing state that the iterator may have and that may make it work more efficiently. Refactor the logic to reseek iterators. This leads to a sizeable speedup with the "reftable" backend: Benchmark 1: update-ref: create many refs (refformat = reftable, preexisting = 100000, new = 10000, revision = HEAD~) Time (mean ± σ): 39.8 ms ± 0.9 ms [User: 29.7 ms, System: 9.8 ms] Range (min … max): 38.4 ms … 42.0 ms 62 runs Benchmark 2: update-ref: create many refs (refformat = reftable, preexisting = 100000, new = 10000, revision = HEAD) Time (mean ± σ): 31.9 ms ± 1.1 ms [User: 27.0 ms, System: 4.5 ms] Range (min … max): 29.8 ms … 34.3 ms 74 runs Summary update-ref: create many refs (refformat = reftable, preexisting = 100000, new = 10000, revision = HEAD) ran 1.25 ± 0.05 times faster than update-ref: create many refs (refformat = reftable, preexisting = 100000, new = 10000, revision = HEAD~) The "files" backend doesn't really show a huge impact: Benchmark 1: update-ref: create many refs (refformat = files, preexisting = 100000, new = 10000, revision = HEAD~) Time (mean ± σ): 392.3 ms ± 7.1 ms [User: 59.7 ms, System: 328.8 ms] Range (min … max): 384.6 ms … 404.5 ms 10 runs Benchmark 2: update-ref: create many refs (refformat = files, preexisting = 100000, new = 10000, revision = HEAD) Time (mean ± σ): 387.7 ms ± 7.4 ms [User: 54.6 ms, System: 329.6 ms] Range (min … max): 377.0 ms … 397.7 ms 10 runs Summary update-ref: create many refs (refformat = files, preexisting = 100000, new = 10000, revision = HEAD) ran 1.01 ± 0.03 times faster than update-ref: create many refs (refformat = files, preexisting = 100000, new = 10000, revision = HEAD~) This is mostly because it is way slower to begin with because it has to create a separate file for each new reference, so the milliseconds we shave off by reseeking the iterator doesn't really translate into a significant relative improvement. Signed-off-by: Patrick Steinhardt --- refs.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/refs.c b/refs.c index 3e65ccad7ac..5f0dbee3aff 100644 --- a/refs.c +++ b/refs.c @@ -2556,8 +2556,13 @@ int refs_verify_refnames_available(struct ref_store *refs, if (!initial_transaction) { int ok; - iter = refs_ref_iterator_begin(refs, dirname.buf, NULL, 0, - DO_FOR_EACH_INCLUDE_BROKEN); + if (!iter) { + iter = refs_ref_iterator_begin(refs, dirname.buf, NULL, 0, + DO_FOR_EACH_INCLUDE_BROKEN); + } else if (ref_iterator_seek(iter, dirname.buf) < 0) { + goto cleanup; + } + while ((ok = ref_iterator_advance(iter)) == ITER_OK) { if (skip && string_list_has_string(skip, iter->refname)) @@ -2570,9 +2575,6 @@ int refs_verify_refnames_available(struct ref_store *refs, if (ok != ITER_DONE) BUG("error while iterating over references"); - - ref_iterator_free(iter); - iter = NULL; } extra_refname = find_descendant_ref(dirname.buf, extras, skip);