From patchwork Tue Apr 2 17:29:51 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13614443 Received: from wfout4-smtp.messagingengine.com (wfout4-smtp.messagingengine.com [64.147.123.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 5078D15A4A3 for ; Tue, 2 Apr 2024 17:29:56 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=64.147.123.147 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1712078997; cv=none; b=qoVkqPnFSVRJUZvjKM2z9KRIkdP8BvYizmiEqsHS8e4GegGEDuDINv4+FwPeXkkZlFEfip0f0QyDhWzUSRSUnCeFHrTY5ZMdKYCc4c9/Z+V4nAnTiuPkVSezHXOyGeNBgWufUwbBbXWb+U1gPG2qD8wdObE2nQaO3H11C937DW8= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1712078997; c=relaxed/simple; bh=T8bMKV0ELM3UJ5ea3hhuv3XeHmA/84Wp6Qj7fqrlJ4E=; h=Date:From:To:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=uGJ1m4X5BjvIhGCK6lIon5/2WwneXBs+3OSo2Fj19bnurdl1DqS0zPqI4UfNO7/L4goLadpUMAEw+stWc9fhMhL7dO3fqMfjWXhM9copuR2n+aCi7sgmJK4XQrUHHFlmHOoa0Amv1Qn1Lgs2CCalz2naHhFKDtRAYGoNvulynp8= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none 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=iL+vgvYQ; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=mKZ1xW6z; arc=none smtp.client-ip=64.147.123.147 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none 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="iL+vgvYQ"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="mKZ1xW6z" Received: from compute7.internal (compute7.nyi.internal [10.202.2.48]) by mailfout.west.internal (Postfix) with ESMTP id 991071C000D7 for ; Tue, 2 Apr 2024 13:29:55 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute7.internal (MEProxy); Tue, 02 Apr 2024 13:29:55 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc :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=fm2; t=1712078995; x=1712165395; bh=dfma8u2u2C WS6L0t7pMbfPcLQMtA90cgEETDUHbYPSM=; b=iL+vgvYQPF1yU5nUh1OCIA7unN SRzQ/Jc0xEGt1KPHKh8lvNRLt8T/YJ8yoG2aJHR5NYlffIHY6UdYIuo9nLtZ7fCi sfk4cd89y32Yo4uxKqZWJaXJP9lCbA3NIefVXDd0y8oCVQXu68ga9kSK3DB80VaI zk4+13npHrufTgvBKrmKOuxVRNAaA3upiRTp8zc/wvEQenZdaUM0tywlaeiz0/Ma L5vhZ9xiw4CocW2atMVATPGe8ztAM4mcfYviGBmJfOE8BiOnM8T8+4EIkDiWOQIu NiB8LvwcshCRX5H+LHSD3hE5qR17Gf0cidE5T51A9mDpOhgKZmgzOCUvqXjA== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc: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-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm2; t=1712078995; x=1712165395; bh=dfma8u2u2CWS6L0t7pMbfPcLQMtA 90cgEETDUHbYPSM=; b=mKZ1xW6z6+FxN3h4fVdXmMKwjqrlo9EWXMdGI+yLuYRc STJH3ZpO2xzRJMeyJ8TJP734DJVgXmu+jhakyWbcs+Emn7zGtfAz9B2Xo5nZ3bKC BFCD6C68OPWtn/RkQvBiK65MssBZWg1DrnA5YXhkYmRhjMgIdmPNfsIwlD4tN5f7 u+H3uDtXHZy2n8UvW5nQNS6Iww6Ph7o8F8wlY15cHp0M5c6KI/3Qopu7gNBqFCeG q7A2KR+/jvrsZMjxcg/Efadv0kA2suxmiPjRRYRQ9CUuznBVm4TJ6UmzpuR5t//0 aLGJzDYYbDta0sQnFJyOSIF4bYPOlv70ob0zN5sNnQ== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvledrudefvddguddugecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecunecujfgurhepfffhvffukfhfgggtuggjsehgtd erredttddvnecuhfhrohhmpefrrghtrhhitghkucfuthgvihhnhhgrrhguthcuoehpshes phhkshdrihhmqeenucggtffrrghtthgvrhhnpeehgefhtdefueffheekgfffudelffejtd fhvdejkedthfehvdelgfetgfdvtedthfenucevlhhushhtvghrufhiiigvpedtnecurfgr rhgrmhepmhgrihhlfhhrohhmpehpshesphhkshdrihhm X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA for ; Tue, 2 Apr 2024 13:29:54 -0400 (EDT) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 6dbdedaf (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO) for ; Tue, 2 Apr 2024 17:29:45 +0000 (UTC) Date: Tue, 2 Apr 2024 19:29:51 +0200 From: Patrick Steinhardt To: git@vger.kernel.org Subject: [PATCH 1/9] refs/reftable: fix D/F conflict error message on ref copy Message-ID: <14b4dacd731a7d9c19029cd8a0c3b6170c31ae25.1712078736.git.ps@pks.im> References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: The `write_copy_table()` function is shared between the reftable implementations for renaming and copying refs. The only difference between those two cases is that the rename will also delete the old reference, whereas copying won't. This has resulted in a bug though where we don't properly verify refname availability. When calling `refs_verify_refname_available()`, we always add the old ref name to the list of refs to be skipped when computing availability, which indicates that the name would be available even if it already exists at the current point in time. This is only the right thing to do for renames though, not for copies. The consequence of this bug is quite harmless because the reftable backend has its own checks for D/F conflicts further down in the call stack, and thus we refuse the update regardless of the bug. But all the user gets in this case is an uninformative message that copying the ref has failed, without any further details. Fix the bug and only add the old name to the skip-list in case we rename the ref. Consequently, this error case will now be handled by `refs_verify_refname_available()`, which knows to provide a proper error message. Signed-off-by: Patrick Steinhardt --- refs/reftable-backend.c | 3 ++- t/t0610-reftable-basics.sh | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c index e206d5a073..0358da14db 100644 --- a/refs/reftable-backend.c +++ b/refs/reftable-backend.c @@ -1351,7 +1351,8 @@ static int write_copy_table(struct reftable_writer *writer, void *cb_data) /* * Verify that the new refname is available. */ - string_list_insert(&skip, arg->oldname); + if (arg->delete_old) + string_list_insert(&skip, arg->oldname); ret = refs_verify_refname_available(&arg->refs->base, arg->newname, NULL, &skip, &errbuf); if (ret < 0) { diff --git a/t/t0610-reftable-basics.sh b/t/t0610-reftable-basics.sh index 686781192e..055231a707 100755 --- a/t/t0610-reftable-basics.sh +++ b/t/t0610-reftable-basics.sh @@ -730,6 +730,39 @@ test_expect_success 'reflog: updates via HEAD update HEAD reflog' ' ) ' +test_expect_success 'branch: copying branch with D/F conflict' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + test_commit A && + git branch branch && + cat >expect <<-EOF && + error: ${SQ}refs/heads/branch${SQ} exists; cannot create ${SQ}refs/heads/branch/moved${SQ} + fatal: branch copy failed + EOF + test_must_fail git branch -c branch branch/moved 2>err && + test_cmp expect err + ) +' + +test_expect_success 'branch: moving branch with D/F conflict' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + test_commit A && + git branch branch && + git branch conflict && + cat >expect <<-EOF && + error: ${SQ}refs/heads/conflict${SQ} exists; cannot create ${SQ}refs/heads/conflict/moved${SQ} + fatal: branch rename failed + EOF + test_must_fail git branch -m branch conflict/moved 2>err && + test_cmp expect err + ) +' + test_expect_success 'worktree: adding worktree creates separate stack' ' test_when_finished "rm -rf repo worktree" && git init repo && From patchwork Tue Apr 2 17:29:56 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13614444 Received: from wfhigh5-smtp.messagingengine.com (wfhigh5-smtp.messagingengine.com [64.147.123.156]) (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 10BEB15A487 for ; Tue, 2 Apr 2024 17:29:59 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=64.147.123.156 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1712079001; cv=none; b=uuOKe54KqdCcPujFDyJoh6a73uskQOaicAbY5UhP3AKfSvuZ2R1DIpdLldR4SgMHF/wUUnWLJWfkjT/qp0rzzy0cUeqxtk81bagvstobNStkFKtgDPbgdkrKJTxAD4VB4XOg5VOMMlG8r/vohAcmVWs3Hwfe04vNN1USF3yC/fk= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1712079001; c=relaxed/simple; bh=DL/fJqg5BuBS+IO/Cnubx8rtLEq9Ew1QPm7dQ+ge+80=; h=Date:From:To:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=VFVrPQZr0JZKK/upqPqa+CPEIPykgx3EwORUvQkpIPKEpid7FJ7HMSolYM7KxJKHll9Xr/b6atuOeVNkqP5VhhVLOPNMiGTfO5Xick4koSraTYvptaFkrlpNE2RkL/HYZtkODNvEtDL4iksdvFVXeNbWL2emHMWnkeZQVmwUgKQ= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none 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=ky2i0CIN; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=htSBE1mQ; arc=none smtp.client-ip=64.147.123.156 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none 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="ky2i0CIN"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="htSBE1mQ" Received: from compute7.internal (compute7.nyi.internal [10.202.2.48]) by mailfhigh.west.internal (Postfix) with ESMTP id 51C0E18000A6 for ; Tue, 2 Apr 2024 13:29:59 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute7.internal (MEProxy); Tue, 02 Apr 2024 13:29:59 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc :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=fm2; t=1712078998; x=1712165398; bh=J+dn4g0C73 LhQUEMahsPUesir3d5EZiFGQnTsRcm4lM=; b=ky2i0CIN2Od0hoQrwMLMwv90uy PaPu5LKyvit8lX6K5zWCqlfgpBTJNknjmPloaE97lLN4ti2cP69lLOZY0CEo+W1v iZVoQhNadYVvyGKx7sScIvmyje6FQUk+B/dJPhjsxxwD+SbiMB+i69PByJcd84IS vcmUVAakcX6mZcn6ufT6NNT/EP78iDS6LzFpfrrY73HGLZkBv9cziGyqa9YYKNzc 5ev/ZuwUdU8TJY5QKhRQcDgadBO7Q2Eojy6vzyrzgsLIoJPiX1rTseJX0s2BM6k5 mYkf8ZCn+N79jIw2m3SNYKb2cml9y6j7ZpiL+uJ3+fwCPL2JCPsP4PBC0YCw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc: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-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm2; t=1712078998; x=1712165398; bh=J+dn4g0C73LhQUEMahsPUesir3d5 EZiFGQnTsRcm4lM=; b=htSBE1mQ1Ve1xWO4T5ApvM28/YE6fr2rB5ZZyVzcIrKJ EJ4cnqqG06J2hynhCpafEfhiyWdf1pY2fLPC645o8FiNttY17H1s2psg3Hwpumaf xJ+2D9GGPNgM3SfibW/YOPIhwatlMLNFMZR1ah6LByEqHiKyVXM6vwln+JjyUToe uIABhMqzM0IdKg3dVXOTaCX80/UvqiLUCuzfeS55KCJl9j6FScMwTOYzippK2BUu TkJMd6ojWGMBC14WjJG+f2xE/Xo0uG1dJ5Bk1EdFYWPtTqPg77celsfH+ExVEiX8 hdc5AtQ3vvselMOchzTGtPGIMBxpp++ASxwMLwljVA== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvledrudefvddguddugecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecunecujfgurhepfffhvffukfhfgggtuggjsehgtd erredttddvnecuhfhrohhmpefrrghtrhhitghkucfuthgvihhnhhgrrhguthcuoehpshes phhkshdrihhmqeenucggtffrrghtthgvrhhnpeehgefhtdefueffheekgfffudelffejtd fhvdejkedthfehvdelgfetgfdvtedthfenucevlhhushhtvghrufhiiigvpedunecurfgr rhgrmhepmhgrihhlfhhrohhmpehpshesphhkshdrihhm X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA for ; Tue, 2 Apr 2024 13:29:58 -0400 (EDT) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 77ac0417 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO) for ; Tue, 2 Apr 2024 17:29:49 +0000 (UTC) Date: Tue, 2 Apr 2024 19:29:56 +0200 From: Patrick Steinhardt To: git@vger.kernel.org Subject: [PATCH 2/9] refs/reftable: perform explicit D/F check when writing symrefs Message-ID: <55db366e61c9292fef0e1d0c61be1da105023bab.1712078736.git.ps@pks.im> References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: We already perform explicit D/F checks in all reftable callbacks which write refs, except when writing symrefs. For one this leads to an error message which isn't perfectly actionable because we only tell the user that there was a D/F conflict, but not which refs conflicted with each other. But second, once all ref updating callbacks explicitly check for D/F conflicts, we can disable the D/F checks in the reftable library itself and thus avoid some duplicated efforts. Refactor the code that writes symref tables to explicitly call into `refs_verify_refname_available()` when writing symrefs. Signed-off-by: Patrick Steinhardt --- refs/reftable-backend.c | 20 +++++++++++++++++--- t/t0610-reftable-basics.sh | 2 +- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c index 0358da14db..8a54b0d8b2 100644 --- a/refs/reftable-backend.c +++ b/refs/reftable-backend.c @@ -1217,6 +1217,7 @@ static int reftable_be_pack_refs(struct ref_store *ref_store, struct write_create_symref_arg { struct reftable_ref_store *refs; struct reftable_stack *stack; + struct strbuf *err; const char *refname; const char *target; const char *logmsg; @@ -1239,6 +1240,11 @@ static int write_create_symref_table(struct reftable_writer *writer, void *cb_da reftable_writer_set_limits(writer, ts, ts); + ret = refs_verify_refname_available(&create->refs->base, create->refname, + NULL, NULL, create->err); + if (ret < 0) + return ret; + ret = reftable_writer_add_ref(writer, &ref); if (ret) return ret; @@ -1280,12 +1286,14 @@ static int reftable_be_create_symref(struct ref_store *ref_store, struct reftable_ref_store *refs = reftable_be_downcast(ref_store, REF_STORE_WRITE, "create_symref"); struct reftable_stack *stack = stack_for(refs, refname, &refname); + struct strbuf err = STRBUF_INIT; struct write_create_symref_arg arg = { .refs = refs, .stack = stack, .refname = refname, .target = target, .logmsg = logmsg, + .err = &err, }; int ret; @@ -1301,9 +1309,15 @@ static int reftable_be_create_symref(struct ref_store *ref_store, done: assert(ret != REFTABLE_API_ERROR); - if (ret) - error("unable to write symref for %s: %s", refname, - reftable_error_str(ret)); + if (ret) { + if (err.len) + error("%s", err.buf); + else + error("unable to write symref for %s: %s", refname, + reftable_error_str(ret)); + } + + strbuf_release(&err); return ret; } diff --git a/t/t0610-reftable-basics.sh b/t/t0610-reftable-basics.sh index 055231a707..12b0004781 100755 --- a/t/t0610-reftable-basics.sh +++ b/t/t0610-reftable-basics.sh @@ -255,7 +255,7 @@ test_expect_success 'ref transaction: creating symbolic ref fails with F/D confl git init repo && test_commit -C repo A && cat >expect <<-EOF && - error: unable to write symref for refs/heads: file/directory conflict + error: ${SQ}refs/heads/main${SQ} exists; cannot create ${SQ}refs/heads${SQ} EOF test_must_fail git -C repo symbolic-ref refs/heads refs/heads/foo 2>err && test_cmp expect err From patchwork Tue Apr 2 17:30:00 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13614445 Received: from wfout4-smtp.messagingengine.com (wfout4-smtp.messagingengine.com [64.147.123.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 AAA0714AD3D for ; Tue, 2 Apr 2024 17:30:04 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=64.147.123.147 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1712079006; cv=none; b=tcjdANnlnHteyaqGzQ3OQcnDj+9VcIZ9ncEaDHvbZS34cX4gqoybRHz3naaajRM6ctm/TzaeUKAqTL2bOGlxSrzeAXTCRCGshZSPB4l06Rbz79xuGwGRCrAmNi3Mj44OLtMqZBfrUwB8Aa7KhEwYzw77N+kF/X5Rjscdks8eE7s= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1712079006; c=relaxed/simple; bh=TUHku8Q2YcBw/+dfwxi4uuOkOWrE51gh+b8XfjtaGmI=; h=Date:From:To:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=HDae/XFeA9jdzQqGe8HF8EJaypNw5FGsXt+ZQ5izRNa9cBbm/vj90j5rDM3t9A/tidjzpFfT4UYV7CM3qCdrsmvXXk2Yx01TaKnF1yf4FrowHJCtrJg4GEGyaFniHiPH1HwIdr6/RKqkaE8QEz73HaI+JFCo4SHxZQPLjQTPZaM= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none 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=X11sNteP; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=eXl8l7Of; arc=none smtp.client-ip=64.147.123.147 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none 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="X11sNteP"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="eXl8l7Of" Received: from compute5.internal (compute5.nyi.internal [10.202.2.45]) by mailfout.west.internal (Postfix) with ESMTP id BD4171C000D7 for ; Tue, 2 Apr 2024 13:30:03 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute5.internal (MEProxy); Tue, 02 Apr 2024 13:30:03 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc :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=fm2; t=1712079003; x=1712165403; bh=whNuf1I48I jBqJw38HGO+tS9NZ2TpvzD3T9r2zAk4GU=; b=X11sNtePMpS3qCRw4XPMntEVZ5 bxop4qGJd7CaR7J8ok6edNS8WrpgT0thyB/b2u9eucj0GIPI5GtYNcJllhjm/Z3o 171UuaItmpagFinI/2H2e7Jt4HX4SKQ2f1DcNiKhM2R8UN6lXF1nv8Pr85N/mM5n KqZfl3h0IAx+76w1lWdylLmY+CeWGSChZdszAMcn/uUbeIJZIWuzK9N7Hze/BX/U vazxtXO2Yra1KdfUVRlY/CcsJwK6bp/VJWnFhYiBiyS8vtV+pt/x1hERYpChl/Ap P1uEExy2nvOaxdx6gAd+6icUCh9LaHYcvdhfT9EYdlnPcU6mJahCrsq6Aq7g== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc: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-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm2; t=1712079003; x=1712165403; bh=whNuf1I48IjBqJw38HGO+tS9NZ2T pvzD3T9r2zAk4GU=; b=eXl8l7OftsG4Bau5HN1F3gY23P/1yofzdbIamTx0gmnF 6fyK6K6QfklbW7f1O7DeaOVhClRIXhGE1RYx4tqrCQcc6BbR0Iz7/uOMrq1i3WPq ukY9yMukAlHoL7tl0gH2U0h3A5Bc8I82sm0SjCZP14aKP2wTJPqAWSmouk/7fwxX nUm1hex963sOWD8HUGxDSjSg3Gm0nQWlgDf6LIJwYyo1bLhZG+ae/uy/NjS+1RtC AJpyu7zDJA1Qyknf1YxGRBlsE/PxWfkZXty77bUb+0cLSuXIdZfFwjweekRjRVOY oaGILV/r/axZm0f14KY5ZeR7kFbXcI+ZeCNqgviOBg== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvledrudefvddguddugecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecunecujfgurhepfffhvffukfhfgggtuggjsehgtd erredttdejnecuhfhrohhmpefrrghtrhhitghkucfuthgvihhnhhgrrhguthcuoehpshes phhkshdrihhmqeenucggtffrrghtthgvrhhnpeehgfejueevjeetudehgffffeffvdejfe ejiedvkeffgfekuefgheevteeufeelkeenucevlhhushhtvghrufhiiigvpedtnecurfgr rhgrmhepmhgrihhlfhhrohhmpehpshesphhkshdrihhm X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA for ; Tue, 2 Apr 2024 13:30:01 -0400 (EDT) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 17fa49b3 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO) for ; Tue, 2 Apr 2024 17:29:53 +0000 (UTC) Date: Tue, 2 Apr 2024 19:30:00 +0200 From: Patrick Steinhardt To: git@vger.kernel.org Subject: [PATCH 3/9] refs/reftable: skip duplicate name checks Message-ID: References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: All the callback functions which write refs in the reftable backend perform D/F conflict checks via `refs_verify_refname_available()`. But in reality we perform these D/F conflict checks a second time in the reftable library via `stack_check_addition()`. Interestingly, the code in the reftable library is inferior compared to the generic function: - It is slower than `refs_verify_refname_available()`, even though this can probably be optimized. - It does not provide a proper error message to the caller, and thus all the user would see is a generic "file/directory conflict" message. Disable the D/F conflict checks in the reftable library by setting the `skip_name_check` write option. This results in a non-negligible speedup when writing many refs. The following benchmark writes 100k refs in a single transaction: Benchmark 1: update-ref: create many refs (HEAD~) Time (mean ± σ): 3.241 s ± 0.040 s [User: 1.854 s, System: 1.381 s] Range (min … max): 3.185 s … 3.454 s 100 runs Benchmark 2: update-ref: create many refs (HEAD) Time (mean ± σ): 2.878 s ± 0.024 s [User: 1.506 s, System: 1.367 s] Range (min … max): 2.838 s … 2.960 s 100 runs Summary update-ref: create many refs (HEAD~) ran 1.13 ± 0.02 times faster than update-ref: create many refs (HEAD) Signed-off-by: Patrick Steinhardt --- refs/reftable-backend.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c index 8a54b0d8b2..7515dd3019 100644 --- a/refs/reftable-backend.c +++ b/refs/reftable-backend.c @@ -247,6 +247,11 @@ static struct ref_store *reftable_be_init(struct repository *repo, refs->write_options.block_size = 4096; refs->write_options.hash_id = repo->hash_algo->format_id; refs->write_options.default_permissions = calc_shared_perm(0666 & ~mask); + /* + * We verify names via `refs_verify_refname_available()`, so there is + * no need to do the same checks in the reftable library again. + */ + refs->write_options.skip_name_check = 1; /* * Set up the main reftable stack that is hosted in GIT_COMMON_DIR. From patchwork Tue Apr 2 17:30:04 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13614446 Received: from wfout4-smtp.messagingengine.com (wfout4-smtp.messagingengine.com [64.147.123.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 5C8A114AD3D for ; Tue, 2 Apr 2024 17:30:08 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=64.147.123.147 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1712079010; cv=none; b=Lyx514gTYm1oeJhkdSXkjDOjpMu+fmQSmaxvNZBDw2IMpkpmPRgkRHCCvt+pIX0zroLoxRbKZf6UjI/XP8ilHQ3Jn38bUa8rm2brxaxoi7PdzG+/XgSOI4IzOcZQJaMxkQUYjH2SawHhpH+2yEiWR273rV1wb/9kC/s6BcybL9M= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1712079010; c=relaxed/simple; bh=kQr4a8pPlaaZdu5HihSqWA31A/nicsIKy3+CP/m7kmw=; h=Date:From:To:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=OBvUHbEoxSrZYo4KZGKrb93AlFbLJuo6i8OYdVcJqHxd7G5jjjvinZlVybs+SSpmrk3GpSE0SNdZNP26kbKX3BYHeSwM7ZF0kDubh8q+yq7B/83o+iua8Guv8Bt1WMX2j/gMh2Fdad9IhlgMFeDe33yJzv4eNGbXKqDf5ZEQfKo= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none 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=fiYMRfxw; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=xJcCy+oc; arc=none smtp.client-ip=64.147.123.147 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none 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="fiYMRfxw"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="xJcCy+oc" Received: from compute2.internal (compute2.nyi.internal [10.202.2.46]) by mailfout.west.internal (Postfix) with ESMTP id 947C21C000E1 for ; Tue, 2 Apr 2024 13:30:07 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute2.internal (MEProxy); Tue, 02 Apr 2024 13:30:07 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc :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=fm2; t=1712079007; x=1712165407; bh=puMETayh41 b31LZ1MXFKgdioV0nfeYLsqpizsAbwo5w=; b=fiYMRfxwUtoPihDvXVP/pMp1Yv HJ+bPKJYMtox6n9A1Vl35krOis543nUyXqpcFd4UZS++MZHVI4GHqareVq7LTDVJ rzxXPuOU1yc0ruOshgWlf8UvYDM95IHQzXIeX7NUFyptYI58T2YT95yMVjApAVN6 0Xp0qxKZnKXL2XCzn9YGoGfczHKaJyu/DDqYiXg6oAR04D44UYyeC+aa/ibUpgm0 1kti6J3pIgO7/JAogM5xdBezBkkHP6IJqvS6AeCP/8PMG6Px4v02j1np5C/7a6Zo DoCrwtRNVG+woeZ45wIHKqomiTrYwsneiO8G8MjMozOlYa9lpQlxLZuDXLZA== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc: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-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm2; t=1712079007; x=1712165407; bh=puMETayh41b31LZ1MXFKgdioV0nf eYLsqpizsAbwo5w=; b=xJcCy+oc2jutH0+qM+vcdcfgk42yfmaakFiNtF63xqOE ITkWj0qpYY/j74BU1no7x4Vnadesy5ej09VIDNVbAwy7CKBAzTGsfA8f0/XEwn6v unxcQF3Bp3w6NBOtJncb2aI5bkje4crgjP2oSJIcn2fWiOrhQZj4Pmp8b3v3lD3O BUM1hyJLPM8XuLOWpXkrzk2vwNHG9HY+T2Y2kcr3ijGqfnONd8KP5O40KxeYNjlx hIgjOrRGxkHuZORjKIJvqSVfzj3aV+DJKsrefh1prwtHCYk9xOIdYFVNMYmIOXYa ruQeYGJUb+jUYb5tDd0xwD0iuNWjL74pT2L2kT9ykg== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvledrudefvddguddugecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecunecujfgurhepfffhvffukfhfgggtuggjsehgtd erredttdejnecuhfhrohhmpefrrghtrhhitghkucfuthgvihhnhhgrrhguthcuoehpshes phhkshdrihhmqeenucggtffrrghtthgvrhhnpefhhffgieejkedthffgjeeukeekhffgue eifeehteegkeeftdeviefffedvjeffleenucffohhmrghinhepuhhpuggrthgvrdhnrghm vgdpuhhpuggrthgvrdgvmhgrihhlpdhuphgurghtvgdrthiipdhuphgurghtvgdrnhgvfi enucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehpshes phhkshdrihhm X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA for ; Tue, 2 Apr 2024 13:30:06 -0400 (EDT) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 00ca721b (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO) for ; Tue, 2 Apr 2024 17:29:57 +0000 (UTC) Date: Tue, 2 Apr 2024 19:30:04 +0200 From: Patrick Steinhardt To: git@vger.kernel.org Subject: [PATCH 4/9] refs/reftable: don't recompute committer ident Message-ID: References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: In order to write reflog entries we need to compute the committer's identity as it becomes encoded in the log record itself. In the reftable backend, computing the identity is repeated for every single reflog entry which we are about to write in a transaction. Needless to say, this can be quite a waste of effort when writing many refs with reflog entries in a single transaction. Refactor the code to pre-compute the committer information. This results in a small speedup when writing 100000 refs in a single transaction: Benchmark 1: update-ref: create many refs (HEAD~) Time (mean ± σ): 2.895 s ± 0.020 s [User: 1.516 s, System: 1.374 s] Range (min … max): 2.868 s … 2.983 s 100 runs Benchmark 2: update-ref: create many refs (HEAD) Time (mean ± σ): 2.845 s ± 0.017 s [User: 1.461 s, System: 1.379 s] Range (min … max): 2.803 s … 2.913 s 100 runs Summary update-ref: create many refs (HEAD) ran 1.02 ± 0.01 times faster than update-ref: create many refs (HEAD~) Signed-off-by: Patrick Steinhardt --- refs/reftable-backend.c | 52 +++++++++++++++++++++++++++-------------- 1 file changed, 34 insertions(+), 18 deletions(-) diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c index 7515dd3019..9e8967e82f 100644 --- a/refs/reftable-backend.c +++ b/refs/reftable-backend.c @@ -171,32 +171,30 @@ static int should_write_log(struct ref_store *refs, const char *refname) } } -static void fill_reftable_log_record(struct reftable_log_record *log) +static void fill_reftable_log_record(struct reftable_log_record *log, const struct ident_split *split) { - const char *info = git_committer_info(0); - struct ident_split split = {0}; + const char *tz_begin; int sign = 1; - if (split_ident_line(&split, info, strlen(info))) - BUG("failed splitting committer info"); - reftable_log_record_release(log); log->value_type = REFTABLE_LOG_UPDATE; log->value.update.name = - xstrndup(split.name_begin, split.name_end - split.name_begin); + xstrndup(split->name_begin, split->name_end - split->name_begin); log->value.update.email = - xstrndup(split.mail_begin, split.mail_end - split.mail_begin); - log->value.update.time = atol(split.date_begin); - if (*split.tz_begin == '-') { + xstrndup(split->mail_begin, split->mail_end - split->mail_begin); + log->value.update.time = atol(split->date_begin); + + tz_begin = split->tz_begin; + if (*tz_begin == '-') { sign = -1; - split.tz_begin++; + tz_begin++; } - if (*split.tz_begin == '+') { + if (*tz_begin == '+') { sign = 1; - split.tz_begin++; + tz_begin++; } - log->value.update.tz_offset = sign * atoi(split.tz_begin); + log->value.update.tz_offset = sign * atoi(tz_begin); } static int read_ref_without_reload(struct reftable_stack *stack, @@ -1023,9 +1021,15 @@ static int write_transaction_table(struct reftable_writer *writer, void *cb_data reftable_stack_merged_table(arg->stack); uint64_t ts = reftable_stack_next_update_index(arg->stack); struct reftable_log_record *logs = NULL; + struct ident_split committer_ident = {0}; size_t logs_nr = 0, logs_alloc = 0, i; + const char *committer_info; int ret = 0; + committer_info = git_committer_info(0); + if (split_ident_line(&committer_ident, committer_info, strlen(committer_info))) + BUG("failed splitting committer info"); + QSORT(arg->updates, arg->updates_nr, transaction_update_cmp); reftable_writer_set_limits(writer, ts, ts); @@ -1091,7 +1095,7 @@ static int write_transaction_table(struct reftable_writer *writer, void *cb_data log = &logs[logs_nr++]; memset(log, 0, sizeof(*log)); - fill_reftable_log_record(log); + fill_reftable_log_record(log, &committer_ident); log->update_index = ts; log->refname = xstrdup(u->refname); memcpy(log->value.update.new_hash, u->new_oid.hash, GIT_MAX_RAWSZ); @@ -1238,9 +1242,11 @@ static int write_create_symref_table(struct reftable_writer *writer, void *cb_da .value.symref = (char *)create->target, .update_index = ts, }; + struct ident_split committer_ident = {0}; struct reftable_log_record log = {0}; struct object_id new_oid; struct object_id old_oid; + const char *committer_info; int ret; reftable_writer_set_limits(writer, ts, ts); @@ -1268,7 +1274,11 @@ static int write_create_symref_table(struct reftable_writer *writer, void *cb_da !should_write_log(&create->refs->base, create->refname)) return 0; - fill_reftable_log_record(&log); + committer_info = git_committer_info(0); + if (split_ident_line(&committer_ident, committer_info, strlen(committer_info))) + BUG("failed splitting committer info"); + + fill_reftable_log_record(&log, &committer_ident); log.refname = xstrdup(create->refname); log.update_index = ts; log.value.update.message = xstrndup(create->logmsg, @@ -1344,10 +1354,16 @@ static int write_copy_table(struct reftable_writer *writer, void *cb_data) struct reftable_log_record old_log = {0}, *logs = NULL; struct reftable_iterator it = {0}; struct string_list skip = STRING_LIST_INIT_NODUP; + struct ident_split committer_ident = {0}; struct strbuf errbuf = STRBUF_INIT; size_t logs_nr = 0, logs_alloc = 0, i; + const char *committer_info; int ret; + committer_info = git_committer_info(0); + if (split_ident_line(&committer_ident, committer_info, strlen(committer_info))) + BUG("failed splitting committer info"); + if (reftable_stack_read_ref(arg->stack, arg->oldname, &old_ref)) { ret = error(_("refname %s not found"), arg->oldname); goto done; @@ -1422,7 +1438,7 @@ static int write_copy_table(struct reftable_writer *writer, void *cb_data) ALLOC_GROW(logs, logs_nr + 1, logs_alloc); memset(&logs[logs_nr], 0, sizeof(logs[logs_nr])); - fill_reftable_log_record(&logs[logs_nr]); + fill_reftable_log_record(&logs[logs_nr], &committer_ident); logs[logs_nr].refname = (char *)arg->newname; logs[logs_nr].update_index = deletion_ts; logs[logs_nr].value.update.message = @@ -1454,7 +1470,7 @@ static int write_copy_table(struct reftable_writer *writer, void *cb_data) */ ALLOC_GROW(logs, logs_nr + 1, logs_alloc); memset(&logs[logs_nr], 0, sizeof(logs[logs_nr])); - fill_reftable_log_record(&logs[logs_nr]); + fill_reftable_log_record(&logs[logs_nr], &committer_ident); logs[logs_nr].refname = (char *)arg->newname; logs[logs_nr].update_index = creation_ts; logs[logs_nr].value.update.message = From patchwork Tue Apr 2 17:30:08 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13614447 Received: from wfhigh5-smtp.messagingengine.com (wfhigh5-smtp.messagingengine.com [64.147.123.156]) (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 EB77414AD3D for ; Tue, 2 Apr 2024 17:30:11 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=64.147.123.156 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1712079013; cv=none; b=TaS2s/3Veh9r7IR8u5cpYkLCQwmr9QJnxFufs3FPTDsLK60yrkczaU2xbJr7jyX7bMVNzVyPbxy5XL1LT8qZrDkWPGriDZWEYDz2Ec0MoGAKxJlVIHBLzHzARq5PPRRiFcdD6ODIx87EflVlFXTXwlYYsnSG07G018s3TH0UpNM= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1712079013; c=relaxed/simple; bh=hypefMVF3/Ogj17LPtUltsQK2JNqmp/pEn8cJMtw/hc=; h=Date:From:To:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=kWLkAKM4BgoHoPBuJBJOC7k8leAkcMJdeGGrIGJ1reJAcQlnE/pzHonr1EiOLmSHP+BqjK6B1ZRto+TEy5+iipKotqd18G1CRUm6CJdRPRP0xWThUziaKawr6VY/PLg/iA+bqbk2pHxO4C2uphgxBLTHlxgRs1hzkXeg/VHQ/es= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none 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=Nk50NIK8; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=VifmtfIy; arc=none smtp.client-ip=64.147.123.156 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none 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="Nk50NIK8"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="VifmtfIy" Received: from compute5.internal (compute5.nyi.internal [10.202.2.45]) by mailfhigh.west.internal (Postfix) with ESMTP id 5795618000B9 for ; Tue, 2 Apr 2024 13:30:11 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute5.internal (MEProxy); Tue, 02 Apr 2024 13:30:11 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc :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=fm2; t=1712079010; x=1712165410; bh=8pOR+nwpbP tyoVnZAX+NmwAlTazeTdbfqfY3cjsMcmI=; b=Nk50NIK8BXPXE2izPtXkXV3yMm D7h9iEc1qoOyrz8ieyTtcNSv4ALSWoWigFO6LsCbmmbkDacBfNMlc1aKhxBMTYF9 xWCdOD0NoN/Gj54TpXHCeq/iOqcpfNLPnXuH6AXKZPOStXAXNm68RA++zcYYtuyf gF05sBWfkhvaufLMAlx21j4XMZc+PkaPFl6BOQOMGHYUWsHbnnCjFgLMdx7Lhpup +XQUCaw/fmNZSK0TG720eBH1L+yaKlmb8X4RWL2hfRLjuBIJHrx4SYiGh9PKMteY bn6gao1HpFgPK8fSHA4edOaehxAr7ZTxuAnUYOfBcRkmHareXa7N+osK2cSw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc: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-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm2; t=1712079010; x=1712165410; bh=8pOR+nwpbPtyoVnZAX+NmwAlTaze TdbfqfY3cjsMcmI=; b=VifmtfIy4XSsDRbxZBVNcQ9wev45zxV1fqwC06uvhoS2 F/2BlSD9EANJF/IYiAXP5IzrAbzES61GcVn1LWRxyUh/npcxWsSGMKUmDctOSJrV OThpZM/Vzn9J2CYXhntesB5V/C2ofmqitefpxEjUNtmhM3SbZ4NHAwXhgsLyJ1Xj tpgwqRfe4BG1bcvyjtl8jqRddX3jnERprMGb2ycYKz3aSGs7pEp+BHLweJP4PcoT sMsM8z6+w8Y/BTUgNiblMC/1jgnNecN+TUpGGN4bx3RHCcjCMGYpc0QlmwNLJryP dEm4CLMmEGPwoGp5uriS00bnooIG8avn1Hr15FDwFw== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvledrudefvddguddugecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecunecujfgurhepfffhvffukfhfgggtuggjsehgtd erredttddvnecuhfhrohhmpefrrghtrhhitghkucfuthgvihhnhhgrrhguthcuoehpshes phhkshdrihhmqeenucggtffrrghtthgvrhhnpeehgefhtdefueffheekgfffudelffejtd fhvdejkedthfehvdelgfetgfdvtedthfenucevlhhushhtvghrufhiiigvpedtnecurfgr rhgrmhepmhgrihhlfhhrohhmpehpshesphhkshdrihhm X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA for ; Tue, 2 Apr 2024 13:30:10 -0400 (EDT) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 44c0ac6a (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO) for ; Tue, 2 Apr 2024 17:30:01 +0000 (UTC) Date: Tue, 2 Apr 2024 19:30:08 +0200 From: Patrick Steinhardt To: git@vger.kernel.org Subject: [PATCH 5/9] reftable/writer: refactorings for `writer_add_record()` Message-ID: <8e9d69e9e6685b097f4d184f9a7e2a2d753c95f1.1712078736.git.ps@pks.im> References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Large parts of the reftable library do not conform to Git's typical code style. Refactor `writer_add_record()` such that it conforms better to it and add some documentation that explains some of its more intricate behaviour. Signed-off-by: Patrick Steinhardt --- reftable/writer.c | 38 +++++++++++++++++++++++++++----------- 1 file changed, 27 insertions(+), 11 deletions(-) diff --git a/reftable/writer.c b/reftable/writer.c index 1d9ff0fbfa..0ad5eb8887 100644 --- a/reftable/writer.c +++ b/reftable/writer.c @@ -209,7 +209,8 @@ static int writer_add_record(struct reftable_writer *w, struct reftable_record *rec) { struct strbuf key = STRBUF_INIT; - int err = -1; + int err; + reftable_record_key(rec, &key); if (strbuf_cmp(&w->last_key, &key) >= 0) { err = REFTABLE_API_ERROR; @@ -218,27 +219,42 @@ static int writer_add_record(struct reftable_writer *w, strbuf_reset(&w->last_key); strbuf_addbuf(&w->last_key, &key); - if (!w->block_writer) { + if (!w->block_writer) writer_reinit_block_writer(w, reftable_record_type(rec)); - } - assert(block_writer_type(w->block_writer) == reftable_record_type(rec)); + if (block_writer_type(w->block_writer) != reftable_record_type(rec)) + BUG("record of type %d added to writer of type %d", + reftable_record_type(rec), block_writer_type(w->block_writer)); - if (block_writer_add(w->block_writer, rec) == 0) { + /* + * Try to add the record to the writer. If this succeeds then we're + * done. Otherwise the block writer may have hit the block size limit + * and needs to be flushed. + */ + if (!block_writer_add(w->block_writer, rec)) { err = 0; goto done; } + /* + * The current block is full, so we need to flush and reinitialize the + * writer to start writing the next block. + */ err = writer_flush_block(w); - if (err < 0) { + if (err < 0) goto done; - } - writer_reinit_block_writer(w, reftable_record_type(rec)); + + /* + * Try to add the record to the writer again. If this still fails then + * the record does not fit into the block size. + * + * TODO: it would be great to have `block_writer_add()` return proper + * error codes so that we don't have to second-guess the failure + * mode here. + */ err = block_writer_add(w->block_writer, rec); - if (err == -1) { - /* we are writing into memory, so an error can only mean it - * doesn't fit. */ + if (err) { err = REFTABLE_ENTRY_TOO_BIG_ERROR; goto done; } From patchwork Tue Apr 2 17:30:12 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13614448 Received: from wfhigh5-smtp.messagingengine.com (wfhigh5-smtp.messagingengine.com [64.147.123.156]) (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 DF8ED14AD3D for ; Tue, 2 Apr 2024 17:30:15 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=64.147.123.156 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1712079017; cv=none; b=CyU9pduVi3kXu3y59cl2xpP0X+1A8h13zardabOz4KdGb4BzEnlcZ5145dmAgEIuRtHxjD5aKX8/ddZ2tBmNW+b1CTEEsJ+f9Qsx5d0ovmyzW9ETOYBe7JVAApusDpDA+Pn3IHGlNWaBy6yI8ctBNfYHxwN5czBxXtwMO0JyzWc= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1712079017; c=relaxed/simple; bh=GNwcJcQm0DLl3n1S8zRvZd2xXpW57WzlT4fS2cP2qbc=; h=Date:From:To:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=iYM0BQ5UUez1jT7fbSIheU8ypn62uvtQe2SuEjWSQDJfu812ubplGA/huJpXfrINDtFrdK59tUQYGkXz73PHxVM23KwbpkRkBkErVGW8SvuXTnFmNpBYBpbxct3Uxau8lc/BOpL0dCBGRGqmLRyvYqt91rx5fa81IHwfptrrCTY= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none 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=Nyzo2NiS; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=gj7qUjwO; arc=none smtp.client-ip=64.147.123.156 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none 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="Nyzo2NiS"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="gj7qUjwO" Received: from compute3.internal (compute3.nyi.internal [10.202.2.43]) by mailfhigh.west.internal (Postfix) with ESMTP id 230FF18000E7 for ; Tue, 2 Apr 2024 13:30:15 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute3.internal (MEProxy); Tue, 02 Apr 2024 13:30:15 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc :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=fm2; t=1712079014; x=1712165414; bh=2J58llVxMk uuH8Gu6gxiN+AQbFMSF7qv3Y5Qy7IDYKM=; b=Nyzo2NiSN0ri56WWNLlknb8LXv D/As3Bkp7556F9ZBrpu8FN1eT5rKiKgMAthMlmEGmHlXUx3nitzKZQ3QZDU54U4z Q5vfofsKYiJJIxhGpu/QjCvSo+mEVQv54dg+idgm3YP6l6zz90o3fu9hs0Bg/MFB VlmWcLpgO7mrTQteRm+o909aVmDmbNrcWiU+WHOZOx2OM3YY2qzD8SD0jw5D29s/ QKx4QKWAHFhW+a4VF0WwwxvZ87hVkyczfn3GJM2hPszKvHjQ5jh8IY4lA1XLSOAY 6uVzwVKgpDDNUCmLZe5X9IxLouECG73UBR9G4VpzJ6pQkauYtzN8WbMoyF3Q== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc: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-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm2; t=1712079014; x=1712165414; bh=2J58llVxMkuuH8Gu6gxiN+AQbFMS F7qv3Y5Qy7IDYKM=; b=gj7qUjwOkj+h7U4sLGNCpvIq6lTbmeUTzCzbrBjx4I0u 4om5NDZ0Qtec9iSXdVZ5XafWpQuCnqdfjzPycLUTZ9uvSD6VHoa5hp0l0pHjYv6S JFW+rrAv3r3UsbSRLXjlOZ0ia1xGRM8mxPUO+aSS89njn03kPX7qSzVmyk3aKifW ckicIRmhrOEIEzty11Z1KSP9ivtiTxuryGdRuxrBbQhRqRV5CnKxqxg+1arOl1nb lDu8U3mTTXlGKua6RLwTW4bZLUaTE0GC1lWL7IQg6V2K0qF6XcNjo2TCtoUJD+DR E7jN2VSOz7muJlrBv+GHY0bew+PgEIB7C8OnJfioFQ== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvledrudefvddguddugecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecunecujfgurhepfffhvffukfhfgggtuggjsehgtd erredttddvnecuhfhrohhmpefrrghtrhhitghkucfuthgvihhnhhgrrhguthcuoehpshes phhkshdrihhmqeenucggtffrrghtthgvrhhnpeehgefhtdefueffheekgfffudelffejtd fhvdejkedthfehvdelgfetgfdvtedthfenucevlhhushhtvghrufhiiigvpedtnecurfgr rhgrmhepmhgrihhlfhhrohhmpehpshesphhkshdrihhm X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA for ; Tue, 2 Apr 2024 13:30:13 -0400 (EDT) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 14af5c5a (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO) for ; Tue, 2 Apr 2024 17:30:05 +0000 (UTC) Date: Tue, 2 Apr 2024 19:30:12 +0200 From: Patrick Steinhardt To: git@vger.kernel.org Subject: [PATCH 6/9] reftable/writer: refactorings for `writer_flush_nonempty_block()` Message-ID: <1f903afdda229ae3e6b73c5612d77f4647079690.1712078736.git.ps@pks.im> References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Large parts of the reftable library do not conform to Git's typical code style. Refactor `writer_flush_nonempty_block()` such that it conforms better to it and add some documentation that explains some of its more intricate behaviour. Signed-off-by: Patrick Steinhardt --- reftable/writer.c | 72 +++++++++++++++++++++++++++++------------------ 1 file changed, 44 insertions(+), 28 deletions(-) diff --git a/reftable/writer.c b/reftable/writer.c index 0ad5eb8887..d347ec4cc6 100644 --- a/reftable/writer.c +++ b/reftable/writer.c @@ -659,58 +659,74 @@ static void writer_clear_index(struct reftable_writer *w) w->index_cap = 0; } -static const int debug = 0; - static int writer_flush_nonempty_block(struct reftable_writer *w) { + struct reftable_index_record index_record = { + .last_key = STRBUF_INIT, + }; uint8_t typ = block_writer_type(w->block_writer); - struct reftable_block_stats *bstats = - writer_reftable_block_stats(w, typ); - uint64_t block_typ_off = (bstats->blocks == 0) ? w->next : 0; - int raw_bytes = block_writer_finish(w->block_writer); - int padding = 0; - int err = 0; - struct reftable_index_record ir = { .last_key = STRBUF_INIT }; + struct reftable_block_stats *bstats; + int raw_bytes, padding = 0, err; + uint64_t block_typ_off; + + /* + * Finish the current block. This will cause the block writer to emit + * restart points and potentially compress records in case we are + * writing a log block. + * + * Note that this is still happening in memory. + */ + raw_bytes = block_writer_finish(w->block_writer); if (raw_bytes < 0) return raw_bytes; - if (!w->opts.unpadded && typ != BLOCK_TYPE_LOG) { + /* + * By default, all records except for log records are padded to the + * block size. + */ + if (!w->opts.unpadded && typ != BLOCK_TYPE_LOG) padding = w->opts.block_size - raw_bytes; - } - if (block_typ_off > 0) { + bstats = writer_reftable_block_stats(w, typ); + block_typ_off = (bstats->blocks == 0) ? w->next : 0; + if (block_typ_off > 0) bstats->offset = block_typ_off; - } - bstats->entries += w->block_writer->entries; bstats->restarts += w->block_writer->restart_len; bstats->blocks++; w->stats.blocks++; - if (debug) { - fprintf(stderr, "block %c off %" PRIu64 " sz %d (%d)\n", typ, - w->next, raw_bytes, - get_be24(w->block + w->block_writer->header_off + 1)); - } - - if (w->next == 0) { + /* + * If this is the first block we're writing to the table then we need + * to also write the reftable header. + */ + if (!w->next) writer_write_header(w, w->block); - } err = padded_write(w, w->block, raw_bytes, padding); if (err < 0) return err; + /* + * Add an index record for every block that we're writing. If we end up + * having more than a threshold of index records we will end up writing + * an index section in `writer_finish_section()`. Each index record + * contains the last record key of the block it is indexing as well as + * the offset of that block. + * + * Note that this also applies when flushing index blocks, in which + * case we will end up with a multi-level index. + */ REFTABLE_ALLOC_GROW(w->index, w->index_len + 1, w->index_cap); - - ir.offset = w->next; - strbuf_reset(&ir.last_key); - strbuf_addbuf(&ir.last_key, &w->block_writer->last_key); - w->index[w->index_len] = ir; - + index_record.offset = w->next; + strbuf_reset(&index_record.last_key); + strbuf_addbuf(&index_record.last_key, &w->block_writer->last_key); + w->index[w->index_len] = index_record; w->index_len++; + w->next += padding + raw_bytes; w->block_writer = NULL; + return 0; } From patchwork Tue Apr 2 17:30:16 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13614449 Received: from wout5-smtp.messagingengine.com (wout5-smtp.messagingengine.com [64.147.123.21]) (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 7DC9614AD3D for ; Tue, 2 Apr 2024 17:30:20 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=64.147.123.21 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1712079022; cv=none; b=BIMyefVs5tPTkFaW4tM5Sd7jnQ+PhhKwN4Sozf6hScGytGAT8o4qGhhjalI5zUTOtW6LoWtMLq20jgxWVMUx5GhcS9e7j6jRk1rtTcW/er0FI405+H8tfIojNBp9XC9XtMQdYyc/FbJ/80WUzGFVkd2tYyGcJWHNAjbZyz+92vs= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1712079022; c=relaxed/simple; bh=oNvdo59gdRlreUsIDGa4sWmcR+O58OIdbW+Cn2ip/mA=; h=Date:From:To:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=HFELgLyACZG4u0n0vL5BJPkXNRh5nl1wfCrVz2gLLi7X72mVaFuRKVWpGTxigtrbwU2jq9CFPpYxFeXeaapYW6QLETUcRn4NgCQgABXxYf925ryix50JTvoZR67FMu0FYFW3gxckoOMaWjJGInA8sj8uPEGneOS3QIuNKuFod/E= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none 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=mLO1fQDm; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=hC3m05rl; arc=none smtp.client-ip=64.147.123.21 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none 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="mLO1fQDm"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="hC3m05rl" Received: from compute5.internal (compute5.nyi.internal [10.202.2.45]) by mailout.west.internal (Postfix) with ESMTP id DADF03200302 for ; Tue, 2 Apr 2024 13:30:19 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute5.internal (MEProxy); Tue, 02 Apr 2024 13:30:20 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc :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=fm2; t=1712079019; x=1712165419; bh=bgXJo5hb6y Kd0qB13tBTisTr3p7+/CTng2jSAdhAaaU=; b=mLO1fQDmfuvBX+g52SXd8sLywS Vs1OChyCVpcgO5MAwMmcrBEUdorY/ypmgxNf41XgGibJpqq+y5Oecw7nwQAQmBG/ 9Ts9/OmVioOctVSI3F9KD022KJSgtzZyj1NADPEdXh2aDKZgR4X1MlicUb8lMV1T d3BTcwkGcIX0JBS3Cb0gAuBK/ecDY3x1EzAoZMl5HMu6dwkrzKurVI5Mg4VWMvSS 8L8u1iJ0FBKAegtZBgVbVipCTzZbl9BmDNKk3HcVXFj0E5b770UvIuqxABbu8lcd wyfgdAtnSxuYxvXGunlo9soiWC+Tvl3yxuRpiGaq2hcDuOAnAoxIoS1DR24Q== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc: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-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm2; t=1712079019; x=1712165419; bh=bgXJo5hb6yKd0qB13tBTisTr3p7+ /CTng2jSAdhAaaU=; b=hC3m05rlK44xVwRG/pXWpgU4ufmcakMb4oQCytU8pj+L ZLqqc/vClXuAnwAzpx645KKkVSLHkYcvxU/CC2mGmfEeNcbhVvd58OSznk7mnxRF Oc78Qi2UvU6SvUsUSR97n8VcKp//OY+fJHAQIxOCOzpgS8LA8bG1FgtWnsVBl99U PPIw/6lrFTfqUIO+vsbqj/fbWuwKdCi7TKO2b4qDc1acRWImqfWmoBJVyOn6cypM oGp4MpKu5ArGwWnMiUOS+siwrwoTbUjjW28u04W01aK0dnhaplH4SXogR8yjjOOn nqeAwESlMJPPMpIByGPimgKEael7tVU4ddHUlvzEeA== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvledrudefvddguddugecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecunecujfgurhepfffhvffukfhfgggtuggjsehgtd erredttddvnecuhfhrohhmpefrrghtrhhitghkucfuthgvihhnhhgrrhguthcuoehpshes phhkshdrihhmqeenucggtffrrghtthgvrhhnpeehgefhtdefueffheekgfffudelffejtd fhvdejkedthfehvdelgfetgfdvtedthfenucevlhhushhtvghrufhiiigvpedunecurfgr rhgrmhepmhgrihhlfhhrohhmpehpshesphhkshdrihhm X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA for ; Tue, 2 Apr 2024 13:30:18 -0400 (EDT) Received: by vm-mail (OpenSMTPD) with ESMTPSA id feacedce (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO) for ; Tue, 2 Apr 2024 17:30:09 +0000 (UTC) Date: Tue, 2 Apr 2024 19:30:16 +0200 From: Patrick Steinhardt To: git@vger.kernel.org Subject: [PATCH 7/9] reftable/block: reuse zstream when writing log blocks Message-ID: <86dab54dfe4501dfa5e50e5a01513c890b62bb4d.1712078736.git.ps@pks.im> References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: While most reftable blocks are written to disk as-is, blocks for log records are compressed with zlib. To compress them we use `compress2()`, which is a simple wrapper around the more complex `zstream` interface that would require multiple function invocations. One downside of this interface is that `compress2()` will reallocate internal state of the `zstream` interface on every single invocation. Consequently, as we call `compress2()` for every single log block which we are about to write, this can lead to quite some memory allocation churn. Refactor the code so that the block writer reuses a `zstream`. This significantly reduces the number of bytes allocated when writing many refs in a single transaction, as demonstrated by the following benchmark that writes 100k refs in a single transaction. Before: HEAP SUMMARY: in use at exit: 671,931 bytes in 151 blocks total heap usage: 22,631,887 allocs, 22,631,736 frees, 1,854,670,793 bytes allocated After: HEAP SUMMARY: in use at exit: 671,931 bytes in 151 blocks total heap usage: 22,620,528 allocs, 22,620,377 frees, 1,245,549,984 bytes allocated Signed-off-by: Patrick Steinhardt --- reftable/block.c | 83 +++++++++++++++++++++++++++++++---------------- reftable/block.h | 1 + reftable/writer.c | 4 +++ 3 files changed, 60 insertions(+), 28 deletions(-) diff --git a/reftable/block.c b/reftable/block.c index e2a2cee58d..1fa74d418f 100644 --- a/reftable/block.c +++ b/reftable/block.c @@ -76,6 +76,10 @@ void block_writer_init(struct block_writer *bw, uint8_t typ, uint8_t *buf, bw->entries = 0; bw->restart_len = 0; bw->last_key.len = 0; + if (!bw->zstream) { + REFTABLE_CALLOC_ARRAY(bw->zstream, 1); + deflateInit(bw->zstream, 9); + } } uint8_t block_writer_type(struct block_writer *bw) @@ -139,39 +143,60 @@ int block_writer_finish(struct block_writer *w) w->next += 2; put_be24(w->buf + 1 + w->header_off, w->next); + /* + * Log records are stored zlib-compressed. Note that the compression + * also spans over the restart points we have just written. + */ if (block_writer_type(w) == BLOCK_TYPE_LOG) { int block_header_skip = 4 + w->header_off; - uLongf src_len = w->next - block_header_skip; - uLongf dest_cap = src_len * 1.001 + 12; - uint8_t *compressed; - - REFTABLE_ALLOC_ARRAY(compressed, dest_cap); - - while (1) { - uLongf out_dest_len = dest_cap; - int zresult = compress2(compressed, &out_dest_len, - w->buf + block_header_skip, - src_len, 9); - if (zresult == Z_BUF_ERROR && dest_cap < LONG_MAX) { - dest_cap *= 2; - compressed = - reftable_realloc(compressed, dest_cap); - if (compressed) - continue; - } - - if (Z_OK != zresult) { - reftable_free(compressed); - return REFTABLE_ZLIB_ERROR; - } - - memcpy(w->buf + block_header_skip, compressed, - out_dest_len); - w->next = out_dest_len + block_header_skip; + uLongf src_len = w->next - block_header_skip, compressed_len; + unsigned char *compressed; + int ret; + + ret = deflateReset(w->zstream); + if (ret != Z_OK) + return REFTABLE_ZLIB_ERROR; + + /* + * Precompute the upper bound of how many bytes the compressed + * data may end up with. Combined with `Z_FINISH`, `deflate()` + * is guaranteed to return `Z_STREAM_END`. + */ + compressed_len = deflateBound(w->zstream, src_len); + REFTABLE_ALLOC_ARRAY(compressed, compressed_len); + + w->zstream->next_out = compressed; + w->zstream->avail_out = compressed_len; + w->zstream->next_in = w->buf + block_header_skip; + w->zstream->avail_in = src_len; + + /* + * We want to perform all decompression in a single + * step, which is why we can pass Z_FINISH here. Note + * that both `Z_OK` and `Z_BUF_ERROR` indicate that we + * need to retry according to documentation. + * + * If the call fails we retry with a bigger output + * buffer. + */ + ret = deflate(w->zstream, Z_FINISH); + if (ret != Z_STREAM_END) { reftable_free(compressed); - break; + return REFTABLE_ZLIB_ERROR; } + + /* + * Overwrite the uncompressed data we have already written and + * adjust the `next` pointer to point right after the + * compressed data. + */ + memcpy(w->buf + block_header_skip, compressed, + w->zstream->total_out); + w->next = w->zstream->total_out + block_header_skip; + + reftable_free(compressed); } + return w->next; } @@ -425,6 +450,8 @@ int block_reader_seek(struct block_reader *br, struct block_iter *it, void block_writer_release(struct block_writer *bw) { + deflateEnd(bw->zstream); + FREE_AND_NULL(bw->zstream); FREE_AND_NULL(bw->restarts); strbuf_release(&bw->last_key); /* the block is not owned. */ diff --git a/reftable/block.h b/reftable/block.h index 47acc62c0a..1375957fc8 100644 --- a/reftable/block.h +++ b/reftable/block.h @@ -18,6 +18,7 @@ license that can be found in the LICENSE file or at * allocation overhead. */ struct block_writer { + z_stream *zstream; uint8_t *buf; uint32_t block_size; diff --git a/reftable/writer.c b/reftable/writer.c index d347ec4cc6..51e663bb19 100644 --- a/reftable/writer.c +++ b/reftable/writer.c @@ -153,6 +153,10 @@ void reftable_writer_free(struct reftable_writer *w) { if (!w) return; + if (w->block_writer) { + block_writer_release(w->block_writer); + w->block_writer = NULL; + } reftable_free(w->block); reftable_free(w); } From patchwork Tue Apr 2 17:30:20 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13614450 Received: from wfhigh5-smtp.messagingengine.com (wfhigh5-smtp.messagingengine.com [64.147.123.156]) (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 5BB6A14AD3D for ; Tue, 2 Apr 2024 17:30:24 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=64.147.123.156 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1712079025; cv=none; b=Et8Vx+V8qad9but0ws5KP5AypKjmrx8vUlyAq8XhNPZpcLwKCQDBgOe+sOjs/E8h1CnSSvYZd9x1+xflQabjmDp5UqUTlt0XbVCLifrqleWC+LUk5AP4u4cvlZ16aUqiRlcHBVtg2a8Jdb3SpuoRVsT7H1g3Nh8HbcHKyRsuOS8= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1712079025; c=relaxed/simple; bh=xFBnPp281z5tn3rvtFHkuC/4AFW9vcVurDAtURzLrCg=; h=Date:From:To:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=fInZnH0h7elunMCHDLqiYvPcxb7Yz/6cZjTnivIVd71BIQmE4bN9Kt4tlKy4Oy9PrcXWz52QP96eHCRgWYiyccRvtrAtL5DlqP4EWyl5ZFMZ/np91/rZ+lWd/HqzHTtzX8bvilsCvqF+gNCY+IR9dESNu3e3sNBMmGtC4GTQHrI= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none 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=bnJW6q4L; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=nOQL6i1f; arc=none smtp.client-ip=64.147.123.156 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none 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="bnJW6q4L"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="nOQL6i1f" Received: from compute7.internal (compute7.nyi.internal [10.202.2.48]) by mailfhigh.west.internal (Postfix) with ESMTP id 972EE18000F1 for ; Tue, 2 Apr 2024 13:30:23 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute7.internal (MEProxy); Tue, 02 Apr 2024 13:30:23 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc :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=fm2; t=1712079023; x=1712165423; bh=AY2SdgYTUB gdYLJogKkCudsfrWpaiS9QWzv7CrYMjK8=; b=bnJW6q4Lf0FLS0aNTkQaYaJ71C gMOItb+AB6WQ2yGA7nBpVacJkDgUHq69grSOayJ4AdR98uBujavSk96jp+9f5CFm SfNNCs710omDbJ4yYVeVGWlziN+/tCjXVjOArKdwX0arXlxZrlZ6mt2/mlqYSudB s1g0zdN4twg12hCBsdZg0vFTd9CUGQmzL/Y4/bhuJ+XN2daAloTuak9FKgJtE2wQ InWsQs/Tx0XtRg8MkU2x+30ogmsMi8Z0puTN31daqjUblxWT+9R7D3e66w0LAWrM I60xz2l2BQGWOg6zKlJk6Raa1iglmcKUdfF63tr/3wfYjc+2udWQs6LmkGyg== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc: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-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm2; t=1712079023; x=1712165423; bh=AY2SdgYTUBgdYLJogKkCudsfrWpa iS9QWzv7CrYMjK8=; b=nOQL6i1fB6R+UGgHRLXUvUsb9V+edTZEq5h87MuDnok/ j/X3/6SyftEsl/ax33xNF36/PYYzRV25x9bo9WvWUZjozs05iEx0U+LxFSspKdcR DsUvv2AQxkC9i8kVI8R/ue3ZWgB21dDv3R+vPiouk850934w7po2H1yDH8yalIHo 2hDtyCKc9EIlCoCdElSIxNlewMhDGXIBCP94Tv0vf3NfQAA8Wr7+VKsYqmphehnY OnEazH6yUyn7bDAxIjhRGoLj7K15iLj2kfJJTJEUHTOY6WXY3bv07TZ0+7xgWPua u6ZlDE/EAI+d0drI30EYyQucYsGf8q/sFU1lkReeaw== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvledrudefvddguddugecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecunecujfgurhepfffhvffukfhfgggtuggjsehgtd erredttddvnecuhfhrohhmpefrrghtrhhitghkucfuthgvihhnhhgrrhguthcuoehpshes phhkshdrihhmqeenucggtffrrghtthgvrhhnpeehgefhtdefueffheekgfffudelffejtd fhvdejkedthfehvdelgfetgfdvtedthfenucevlhhushhtvghrufhiiigvpedvnecurfgr rhgrmhepmhgrihhlfhhrohhmpehpshesphhkshdrihhm X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA for ; Tue, 2 Apr 2024 13:30:22 -0400 (EDT) Received: by vm-mail (OpenSMTPD) with ESMTPSA id dc178624 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO) for ; Tue, 2 Apr 2024 17:30:14 +0000 (UTC) Date: Tue, 2 Apr 2024 19:30:20 +0200 From: Patrick Steinhardt To: git@vger.kernel.org Subject: [PATCH 8/9] reftable/block: reuse compressed array Message-ID: <9899b58dcff6a8380eb7cf3622c7dfff51a10a2c.1712078736.git.ps@pks.im> References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Similar to the preceding commit, let's reuse the `compressed` array that we use to store compressed data in. This results in a small reduction in memory allocations when writing many refs. Before: HEAP SUMMARY: in use at exit: 671,931 bytes in 151 blocks total heap usage: 22,620,528 allocs, 22,620,377 frees, 1,245,549,984 bytes allocated After: HEAP SUMMARY: in use at exit: 671,931 bytes in 151 blocks total heap usage: 22,618,257 allocs, 22,618,106 frees, 1,236,351,528 bytes allocated So while the reduction in allocations isn't really all that big, it's a low hanging fruit and thus there isn't much of a reason not to pick it. Signed-off-by: Patrick Steinhardt --- reftable/block.c | 14 +++++--------- reftable/block.h | 3 +++ 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/reftable/block.c b/reftable/block.c index 1fa74d418f..870d88b58d 100644 --- a/reftable/block.c +++ b/reftable/block.c @@ -150,7 +150,6 @@ int block_writer_finish(struct block_writer *w) if (block_writer_type(w) == BLOCK_TYPE_LOG) { int block_header_skip = 4 + w->header_off; uLongf src_len = w->next - block_header_skip, compressed_len; - unsigned char *compressed; int ret; ret = deflateReset(w->zstream); @@ -163,9 +162,9 @@ int block_writer_finish(struct block_writer *w) * is guaranteed to return `Z_STREAM_END`. */ compressed_len = deflateBound(w->zstream, src_len); - REFTABLE_ALLOC_ARRAY(compressed, compressed_len); + REFTABLE_ALLOC_GROW(w->compressed, compressed_len, w->compressed_cap); - w->zstream->next_out = compressed; + w->zstream->next_out = w->compressed; w->zstream->avail_out = compressed_len; w->zstream->next_in = w->buf + block_header_skip; w->zstream->avail_in = src_len; @@ -180,21 +179,17 @@ int block_writer_finish(struct block_writer *w) * buffer. */ ret = deflate(w->zstream, Z_FINISH); - if (ret != Z_STREAM_END) { - reftable_free(compressed); + if (ret != Z_STREAM_END) return REFTABLE_ZLIB_ERROR; - } /* * Overwrite the uncompressed data we have already written and * adjust the `next` pointer to point right after the * compressed data. */ - memcpy(w->buf + block_header_skip, compressed, + memcpy(w->buf + block_header_skip, w->compressed, w->zstream->total_out); w->next = w->zstream->total_out + block_header_skip; - - reftable_free(compressed); } return w->next; @@ -453,6 +448,7 @@ void block_writer_release(struct block_writer *bw) deflateEnd(bw->zstream); FREE_AND_NULL(bw->zstream); FREE_AND_NULL(bw->restarts); + FREE_AND_NULL(bw->compressed); strbuf_release(&bw->last_key); /* the block is not owned. */ } diff --git a/reftable/block.h b/reftable/block.h index 1375957fc8..657498014c 100644 --- a/reftable/block.h +++ b/reftable/block.h @@ -19,6 +19,9 @@ license that can be found in the LICENSE file or at */ struct block_writer { z_stream *zstream; + unsigned char *compressed; + size_t compressed_cap; + uint8_t *buf; uint32_t block_size; From patchwork Tue Apr 2 17:30:24 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13614451 Received: from wfhigh5-smtp.messagingengine.com (wfhigh5-smtp.messagingengine.com [64.147.123.156]) (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 E89C614AD3D for ; Tue, 2 Apr 2024 17:30:28 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=64.147.123.156 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1712079030; cv=none; b=D0ZlEZBuNZPNfxhH0wovWtl4p0QQxTSIclRI+eom55NZR0jmeQDJCOb3uJMT1V1l9HUm44aGrvPE3U4yOUJ3HX29WpO7Ro8OhQB4vQwDl0W+kJn7SsL9fUCT3F5uomodRlXu7N9496Sq+HfuzqqGf1SS28ZI0GbAWln+KhirZUU= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1712079030; c=relaxed/simple; bh=G+BSWqxhIuTM7xTDpoIm0YVo8nh3LsFhhQ76RBlq0IA=; h=Date:From:To:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=Bb6XN8qN2OyE5eKhHKTeWGIYuXs6edymPI3iIur9ToSsfYQFhsGBNHlsbJpiJgPW48ouE8ozLRRsW67G13oHbG0Gp/J+pXu/S8UNyY8jtNCbemsQjHZdNt5gpAjAqmdOHECjv50/+TJWBIcOsD5aI2Toa7pFKypHVNGmBHIfrdw= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none 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=cxJLW8bO; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=JVddd6he; arc=none smtp.client-ip=64.147.123.156 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none 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="cxJLW8bO"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="JVddd6he" Received: from compute7.internal (compute7.nyi.internal [10.202.2.48]) by mailfhigh.west.internal (Postfix) with ESMTP id 57A1B18000B9 for ; Tue, 2 Apr 2024 13:30:28 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute7.internal (MEProxy); Tue, 02 Apr 2024 13:30:28 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc :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=fm2; t=1712079027; x=1712165427; bh=B0LIe+bm9S lO45oMW70huwR/+SRnXMLTOu8FAgBngV8=; b=cxJLW8bOvLSDEGF+FCTjfwrmnf dqReA+oX3fY+cg4UOH4pa1xOYptRo3FSKHw9Gj4i+lvJx4JE4GVcHpmw/GR0WW5G wXs8x2jLpG/xCpiPnqGgH6zKUq4HS3rLd5L5qhoMFdfql5X87Ps5iDCjUXs/nmx4 FZLhfVWBPQGF4Z/M8smlNcslhfxe5b2FISpOlHIRlkOk1cImqhHB7tZDPAlJW3kW RZUQN/FGQeqA/Ns7RkSm8GmeCPlvHxe/aKSjy6xKz5QTPa7PnGSdSvNeqD+mQ8th dJwLCbI739KqXcuO/H1rwwBBA5A9W8VbH4zalhsejx7MM1iPhf/XWV18+kbA== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc: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-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm2; t=1712079027; x=1712165427; bh=B0LIe+bm9SlO45oMW70huwR/+SRn XMLTOu8FAgBngV8=; b=JVddd6hev0HHjReUwaGa6WGoPxV/upAaqGRGQFtA+3Mp BX0ik9GQVho0+RqMpn30/rRS4Uebvq0FF/UYxDmQI3R+iksmHELFYXG8JjjmTOh+ CMhp934BxrWcBpqbt2jW9Ki3iOiXWdwDuEfvr5hB9HrUGEfIqR/lVfMolmLwVCq1 PC51Gf2zYDPkxneNFUUZ03uf5U+Da0w7iK3+SRyYZy6RNyDgGLDorvgsN72gEUxW Rjh3VCxIkHpOsLnJAI4OZP07lhh9lYGfM6ehObzmUsRhyf4aFMQIlnDoXPZbM2ax 8DzOtbwQrZZ0G2fXUMB+KVjshFEud921FqPoFhPCWw== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvledrudefvddguddugecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecunecujfgurhepfffhvffukfhfgggtuggjsehgtd erredttddvnecuhfhrohhmpefrrghtrhhitghkucfuthgvihhnhhgrrhguthcuoehpshes phhkshdrihhmqeenucggtffrrghtthgvrhhnpeehgefhtdefueffheekgfffudelffejtd fhvdejkedthfehvdelgfetgfdvtedthfenucevlhhushhtvghrufhiiigvpedvnecurfgr rhgrmhepmhgrihhlfhhrohhmpehpshesphhkshdrihhm X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA for ; Tue, 2 Apr 2024 13:30:27 -0400 (EDT) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 083619c5 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO) for ; Tue, 2 Apr 2024 17:30:18 +0000 (UTC) Date: Tue, 2 Apr 2024 19:30:24 +0200 From: Patrick Steinhardt To: git@vger.kernel.org Subject: [PATCH 9/9] reftable/writer: reset `last_key` instead of releasing it Message-ID: <6950ae4ea758a92ba87f73d5ce66c5dfb896234c.1712078736.git.ps@pks.im> References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: The reftable writer tracks the last key that it has written so that it can properly compute the compressed prefix for the next record it is about to write. This last key must be reset whenever we move on to write the next block, which is done in `writer_reinit_block_writer()`. We do this by calling `strbuf_release()` though, which needlessly deallocates the underlying buffer. Convert the code to use `strbuf_reset()` instead, which saves one allocation per block we're about to write. This requires us to also amend `reftable_writer_free()` to release the buffer's memory now as we previously seemingly relied on `writer_reinit_block_writer()` to release the memory for us. Releasing memory here is the right thing to do anyway. While at it, convert a callsite where we truncate the buffer by setting its length to zero to instead use `strbuf_reset()`, too. Signed-off-by: Patrick Steinhardt --- reftable/writer.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/reftable/writer.c b/reftable/writer.c index 51e663bb19..4b3a4e3f3c 100644 --- a/reftable/writer.c +++ b/reftable/writer.c @@ -109,7 +109,7 @@ static void writer_reinit_block_writer(struct reftable_writer *w, uint8_t typ) block_start = header_size(writer_version(w)); } - strbuf_release(&w->last_key); + strbuf_reset(&w->last_key); block_writer_init(&w->block_writer_data, typ, w->block, w->opts.block_size, block_start, hash_size(w->opts.hash_id)); @@ -157,6 +157,7 @@ void reftable_writer_free(struct reftable_writer *w) block_writer_release(w->block_writer); w->block_writer = NULL; } + strbuf_release(&w->last_key); reftable_free(w->block); reftable_free(w); } @@ -472,7 +473,7 @@ static int writer_finish_section(struct reftable_writer *w) bstats->max_index_level = max_level; /* Reinit lastKey, as the next section can start with any key. */ - w->last_key.len = 0; + strbuf_reset(&w->last_key); return 0; }