From patchwork Wed Jan 3 06:22: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: 13509677 Received: from out3-smtp.messagingengine.com (out3-smtp.messagingengine.com [66.111.4.27]) (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 7E54717998 for ; Wed, 3 Jan 2024 06:22:14 +0000 (UTC) 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="NPZh+oNl"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="ltzz9qel" Received: from compute2.internal (compute2.nyi.internal [10.202.2.46]) by mailout.nyi.internal (Postfix) with ESMTP id 4BDD25C019A; Wed, 3 Jan 2024 01:22:13 -0500 (EST) Received: from mailfrontend2 ([10.202.2.163]) by compute2.internal (MEProxy); Wed, 03 Jan 2024 01:22:13 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc: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=1704262933; x=1704349333; bh=OWb/LKHNxU hBEsaFbJkRcMU8i7jJwRwufCiKMNc20p0=; b=NPZh+oNlkCk1iB/zA8ce25Xqcm es5Xr/BG7Ms8hEvga3evHArJ8fl+WkfS2/F4NUL1Bt9UA6CbQlYYzenNFPvK3zeT zPnhSdY6LScZyIlZ83ESEu1uQLqA+JTFFyREwe8ugLnLrEnBTkQ+UVcjXrR5E4dm Op4E7B9EmAgoDe5VTX5/rWfvYCjuKAlY7n0fsqCHtWBuFtsLBqM5Vn5cm82EVtio 2i7cLO/vU1mD4Qw5qb/FPdqvpF4nMiEt3tDweeAoFxgP1w3IPVsTZpANcRAsL+GE wr8LVpD1ysKHHIe0zDKX6Zo35XgOxkoPRNViacO/AmqJPgNv7UjRCA/Oy2qg== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc: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=1704262933; x=1704349333; bh=OWb/LKHNxUhBEsaFbJkRcMU8i7jJ wRwufCiKMNc20p0=; b=ltzz9qelZ51AYqXtVVrhGdvZQNbkl6bJNPx6lsYgQ2Nb 9fSNtY/6K80OHgmqQO6Apuv737UWp1WocEg8Bz7kLRxpBcLfZq0d0SN0+LVZsv5A guFr6mevXEVtnRnzvL0iSZdz9YaICxXHXBTp2U4Uh2AmpfWOXzSsyAcinyDrKH3D GQWp6QP7bgZ9CfbHIWfEHn9kFNrEUYk8+KdX8cZO5vvuRjPsxUA4pSc4+Wpq3RPK wryK3J4dl5fl44K+zuaoC9UuBUhh1+VeZqQHCxkkpPnAORdHppc38K4t59ebVCJF 7xYSLstSsEKXgwjBhw8IW8Z1rApwniqIENu8A1/N6w== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvkedrvdeggedgleeiucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucenucfjughrpeffhffvvefukfhfgggtuggjsehgtd erredttddvnecuhfhrohhmpefrrghtrhhitghkucfuthgvihhnhhgrrhguthcuoehpshes phhkshdrihhmqeenucggtffrrghtthgvrhhnpeeukedtvedtffevleejtefgheehieegke eluddvfeefgeehgfeltddtheejleffteenucevlhhushhtvghrufhiiigvpedtnecurfgr rhgrmhepmhgrihhlfhhrohhmpehpshesphhkshdrihhm X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Wed, 3 Jan 2024 01:22:12 -0500 (EST) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 283d0736 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Wed, 3 Jan 2024 06:19:45 +0000 (UTC) Date: Wed, 3 Jan 2024 07:22:08 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Han-Wen Nienhuys , Junio C Hamano Subject: [PATCH v3 0/8] reftable: fixes and optimizations (pt.2) 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: Hi, this is the third version of my patch series that contains additional fixes and optimizations for the reftable library. The only changes in this iteration are improvements to the commit messages to hopefully make them easier to understand. Thanks Junio for your suggestions! Patrick Patrick Steinhardt (8): reftable/stack: do not overwrite errors when compacting reftable/stack: do not auto-compact twice in `reftable_stack_add()` reftable/writer: fix index corruption when writing multiple indices reftable/record: constify some parts of the interface reftable/record: store "val1" hashes as static arrays reftable/record: store "val2" hashes as static arrays reftable/merged: really reuse buffers to compute record keys reftable/merged: transfer ownership of records when iterating reftable/block_test.c | 4 +- reftable/merged.c | 8 +-- reftable/merged_test.c | 16 +++--- reftable/readwrite_test.c | 102 +++++++++++++++++++++++++++++++------ reftable/record.c | 17 ++----- reftable/record_test.c | 5 -- reftable/reftable-record.h | 11 ++-- reftable/stack.c | 23 +++------ reftable/stack_test.c | 2 - reftable/writer.c | 4 +- 10 files changed, 117 insertions(+), 75 deletions(-) Range-diff against v2: 1: 22a57020c6 = 1: 1dc8ddf04a reftable/stack: do not overwrite errors when compacting 2: a08f7efc99 = 2: eccc34a4b6 reftable/stack: do not auto-compact twice in `reftable_stack_add()` 3: c00e08d97f = 3: 15e12b8f29 reftable/writer: fix index corruption when writing multiple indices 4: 3416268087 = 4: 017e8943c7 reftable/record: constify some parts of the interface 5: 46ca3a37f8 ! 5: f1d2190489 reftable/record: store "val1" hashes as static arrays @@ Metadata ## Commit message ## reftable/record: store "val1" hashes as static arrays - When reading ref records of type "val1" we store its object ID in an + When reading ref records of type "val1", we store its object ID in an allocated array. This results in an additional allocation for every single ref record we read, which is rather inefficient especially when iterating over refs. - Refactor the code to instead use a static array of `GIT_MAX_RAWSZ` + Refactor the code to instead use an embedded array of `GIT_MAX_RAWSZ` bytes. While this means that `struct ref_record` is bigger now, we typically do not store all refs in an array anyway and instead only handle a limited number of records at the same point in time. 6: c8a36917b1 = 6: 6ec02d6775 reftable/record: store "val2" hashes as static arrays 7: 6313f8affd ! 7: 845dec2390 reftable/merged: really reuse buffers to compute record keys @@ Commit message reftable/merged: really reuse buffers to compute record keys In 829231dc20 (reftable/merged: reuse buffer to compute record keys, - 2023-12-11), we have refactored the merged iterator to reuse a set of - buffers that it would otherwise have to reallocate on every single - iteration. Unfortunately, there was a brown-paper-bag-style bug here as - we continue to release these buffers after the iteration, and thus we - have essentially gained nothing. + 2023-12-11), we have refactored the merged iterator to reuse a pair of + long-living strbufs by relying on the fact that `reftable_record_key()` + tries to reuse already allocated strbufs by calling `strbuf_reset()`, + which should give us significantly fewer reallocations compared to the + old code that used on-stack strbufs that are allocated for each and + every iteration. Unfortunately, we called `strbuf_release()` on these + long-living strbufs that we meant to reuse on each iteration, defeating + the optimization. Fix this performance issue by not releasing those buffers on iteration anymore, where we instead rely on `merged_iter_close()` to release the 8: 25a3919e58 ! 8: eea0e161ad reftable/merged: transfer ownership of records when iterating @@ Metadata ## Commit message ## reftable/merged: transfer ownership of records when iterating - When iterating over recods with the merged iterator we put the records + When iterating over records with the merged iterator we put the records into a priority queue before yielding them to the caller. This means that we need to allocate the contents of these records before we can pass them over to the caller. base-commit: a26002b62827b89a19b1084bd75d9371d565d03c