git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: git@vger.kernel.org
Subject: [PATCH 5/5] cat-file: use packed_object_info() for --batch-all-objects
Date: Tue, 5 Oct 2021 16:38:04 -0400	[thread overview]
Message-ID: <YVy3rPuUal0+9iJs@coredump.intra.peff.net> (raw)
In-Reply-To: <YVy1sx8Xb1xMLFQT@coredump.intra.peff.net>

When "cat-file --batch-all-objects" iterates over each object, it knows
where to find each one. But when we look up details of the object, we
don't use that information at all.

This patch teaches it to use the pack/offset pair when we're iterating
over objects in a pack. This yields a measurable speed improvement
(timings on a fully packed clone of linux.git):

  Benchmark #1: ./git.old cat-file --batch-all-objects --unordered --batch-check="%(objecttype) %(objectname)"
    Time (mean ± σ):      8.128 s ±  0.118 s    [User: 7.968 s, System: 0.156 s]
    Range (min … max):    8.007 s …  8.301 s    10 runs

  Benchmark #2: ./git.new cat-file --batch-all-objects --unordered --batch-check="%(objecttype) %(objectname)"
    Time (mean ± σ):      4.294 s ±  0.064 s    [User: 4.167 s, System: 0.125 s]
    Range (min … max):    4.227 s …  4.457 s    10 runs

  Summary
    './git.new cat-file --batch-all-objects --unordered --batch-check="%(objecttype) %(objectname)"' ran
      1.89 ± 0.04 times faster than './git.old cat-file --batch-all-objects --unordered --batch-check="%(objecttype) %(objectname)"

The implementation is pretty simple: we just call packed_object_info()
instead of oid_object_info_extended() when we can. Most of the changes
are just plumbing the pack/offset pair through the callstack. There is
one subtlety: replace lookups are not handled by packed_object_info().
But since those are disabled for --batch-all-objects, and since we'll
only have pack info when that option is in effect, we don't have to
worry about that.

There are a few limitations to this optimization which we could address
with further work:

 - I didn't bother recording when we found an object loose. Technically
   this could save us doing a fruitless lookup in the pack index. But
   opening and mmap-ing a loose object is so expensive in the first
   place that this doesn't matter much. And if your repository is large
   enough to care about per-object performance, most objects are going
   to be packed anyway.

 - This works only in --unordered mode. For the sorted mode, we'd have
   to record the pack/offset pair as part of our oid-collection. That's
   more code, plus at least 16 extra bytes of heap per object. It would
   probably still be a net win in runtime, but we'd need to measure.

 - For --batch, this still helps us with getting the object metadata,
   but we still do a from-scratch lookup for the object contents. This
   probably doesn't matter that much, because the lookup cost will be
   much smaller relative to the cost of actually unpacking and printing
   the objects.

   For small objects, we could probably swap out read_object_file() for
   using packed_object_info() with a "object_info.contentp" to get the
   contents. But we'd still need to deal with streaming for larger
   objects. A better path forward here is to teach the initial
   oid_object_info_extended() / packed_object_info() calls to retrieve
   the contents of smaller objects while they are already being
   accessed. That would save the extra lookup entirely. But it's a
   non-trivial feature to add to the object_info code, so I left it for
   now.

Signed-off-by: Jeff King <peff@peff.net>
---
I have some patches for the "give me the content in a single call if
it's small" idea, but they need some polishing. It also doesn't produce
as spectacular a speedup as I'd hoped, which is why I threw it on the
back burner. I think it's just because actually dealing with the object
content is so much more expensive than an extra lookup.

 builtin/cat-file.c | 45 +++++++++++++++++++++++++++++++--------------
 1 file changed, 31 insertions(+), 14 deletions(-)

diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index b533935d5c..219ff5628d 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -358,15 +358,26 @@ static void print_object_or_die(struct batch_options *opt, struct expand_data *d
 static void batch_object_write(const char *obj_name,
 			       struct strbuf *scratch,
 			       struct batch_options *opt,
-			       struct expand_data *data)
+			       struct expand_data *data,
+			       struct packed_git *pack,
+			       off_t offset)
 {
-	if (!data->skip_object_info &&
-	    oid_object_info_extended(the_repository, &data->oid, &data->info,
-				     OBJECT_INFO_LOOKUP_REPLACE) < 0) {
-		printf("%s missing\n",
-		       obj_name ? obj_name : oid_to_hex(&data->oid));
-		fflush(stdout);
-		return;
+	if (!data->skip_object_info) {
+		int ret;
+
+		if (pack)
+			ret = packed_object_info(the_repository, pack, offset,
+						 &data->info);
+		else
+			ret = oid_object_info_extended(the_repository,
+						       &data->oid, &data->info,
+						       OBJECT_INFO_LOOKUP_REPLACE);
+		if (ret < 0) {
+			printf("%s missing\n",
+			       obj_name ? obj_name : oid_to_hex(&data->oid));
+			fflush(stdout);
+			return;
+		}
 	}
 
 	strbuf_reset(scratch);
@@ -428,7 +439,7 @@ static void batch_one_object(const char *obj_name,
 		return;
 	}
 
-	batch_object_write(obj_name, scratch, opt, data);
+	batch_object_write(obj_name, scratch, opt, data, NULL, 0);
 }
 
 struct object_cb_data {
@@ -442,7 +453,8 @@ static int batch_object_cb(const struct object_id *oid, void *vdata)
 {
 	struct object_cb_data *data = vdata;
 	oidcpy(&data->expand->oid, oid);
-	batch_object_write(NULL, data->scratch, data->opt, data->expand);
+	batch_object_write(NULL, data->scratch, data->opt, data->expand,
+			   NULL, 0);
 	return 0;
 }
 
@@ -463,31 +475,36 @@ static int collect_packed_object(const struct object_id *oid,
 	return 0;
 }
 
-static int batch_unordered_object(const struct object_id *oid, void *vdata)
+static int batch_unordered_object(const struct object_id *oid,
+				  struct packed_git *pack, off_t offset,
+				  void *vdata)
 {
 	struct object_cb_data *data = vdata;
 
 	if (oidset_insert(data->seen, oid))
 		return 0;
 
 	oidcpy(&data->expand->oid, oid);
-	batch_object_write(NULL, data->scratch, data->opt, data->expand);
+	batch_object_write(NULL, data->scratch, data->opt, data->expand,
+			   pack, offset);
 	return 0;
 }
 
 static int batch_unordered_loose(const struct object_id *oid,
 				 const char *path,
 				 void *data)
 {
-	return batch_unordered_object(oid, data);
+	return batch_unordered_object(oid, NULL, 0, data);
 }
 
 static int batch_unordered_packed(const struct object_id *oid,
 				  struct packed_git *pack,
 				  uint32_t pos,
 				  void *data)
 {
-	return batch_unordered_object(oid, data);
+	return batch_unordered_object(oid, pack,
+				      nth_packed_object_offset(pack, pos),
+				      data);
 }
 
 static int batch_objects(struct batch_options *opt)
-- 
2.33.0.1231.g45ae28b974

  parent reply	other threads:[~2021-10-05 20:38 UTC|newest]

Thread overview: 101+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-05 20:29 [PATCH 0/5] cat-file replace handling and optimization Jeff King
2021-10-05 20:30 ` [PATCH 1/5] t1006: clean up broken objects Jeff King
2021-10-05 20:31 ` [PATCH 2/5] cat-file: mention --unordered along with --batch-all-objects Jeff King
2021-10-05 21:02   ` Ævar Arnfjörð Bjarmason
2021-10-05 21:41     ` Jeff King
2021-10-06  9:02       ` Ævar Arnfjörð Bjarmason
2021-10-06 16:15         ` Jeff King
2021-10-07 10:18           ` Ævar Arnfjörð Bjarmason
2021-10-08  2:30             ` Jeff King
2021-10-08  7:54               ` Ævar Arnfjörð Bjarmason
2021-10-08 20:34                 ` Junio C Hamano
2021-10-08 21:44                   ` Jeff King
2021-10-08 22:04                     ` Junio C Hamano
2021-11-06 21:46                       ` [PATCH 00/10] cat-file: better usage UX & error messages Ævar Arnfjörð Bjarmason
2021-11-06 21:46                         ` [PATCH 01/10] cat-file tests: test bad usage Ævar Arnfjörð Bjarmason
2021-11-07  1:07                           ` Eric Sunshine
2021-11-06 21:46                         ` [PATCH 02/10] cat-file tests: test messaging on bad objects/paths Ævar Arnfjörð Bjarmason
2021-11-06 21:46                         ` [PATCH 03/10] parse-options API: add a usage_msg_optf() Ævar Arnfjörð Bjarmason
2021-11-06 21:46                         ` [PATCH 04/10] cat-file docs: fix SYNOPSIS and "-h" output Ævar Arnfjörð Bjarmason
2021-11-06 21:46                         ` [PATCH 05/10] cat-file: move "usage" variable to cmd_cat_file() Ævar Arnfjörð Bjarmason
2021-11-06 21:46                         ` [PATCH 06/10] cat-file: make --batch-all-objects a CMDMODE Ævar Arnfjörð Bjarmason
2021-11-07  3:00                           ` Eric Sunshine
2021-11-06 21:46                         ` [PATCH 07/10] cat-file: fix remaining usage bugs Ævar Arnfjörð Bjarmason
2021-11-06 21:47                         ` [PATCH 08/10] cat-file: correct and improve usage information Ævar Arnfjörð Bjarmason
2021-11-06 21:47                         ` [PATCH 09/10] object-name.c: don't have GET_OID_ONLY_TO_DIE imply *_QUIETLY Ævar Arnfjörð Bjarmason
2021-11-07  3:05                           ` Eric Sunshine
2021-11-06 21:47                         ` [PATCH 10/10] cat-file: improve --(textconv|filters) disambiguation Ævar Arnfjörð Bjarmason
2021-11-12 22:19                         ` [PATCH v2 00/10] cat-file: better usage UX & error messages Ævar Arnfjörð Bjarmason
2021-11-12 22:19                           ` [PATCH v2 01/10] cat-file tests: test bad usage Ævar Arnfjörð Bjarmason
2021-11-12 22:20                           ` [PATCH v2 02/10] cat-file tests: test messaging on bad objects/paths Ævar Arnfjörð Bjarmason
2021-11-12 22:20                           ` [PATCH v2 03/10] parse-options API: add a usage_msg_optf() Ævar Arnfjörð Bjarmason
2021-11-12 22:20                           ` [PATCH v2 04/10] cat-file docs: fix SYNOPSIS and "-h" output Ævar Arnfjörð Bjarmason
2021-11-12 22:20                           ` [PATCH v2 05/10] cat-file: move "usage" variable to cmd_cat_file() Ævar Arnfjörð Bjarmason
2021-11-12 22:20                           ` [PATCH v2 06/10] cat-file: make --batch-all-objects a CMDMODE Ævar Arnfjörð Bjarmason
2021-11-12 22:20                           ` [PATCH v2 07/10] cat-file: fix remaining usage bugs Ævar Arnfjörð Bjarmason
2021-11-12 22:20                           ` [PATCH v2 08/10] cat-file: correct and improve usage information Ævar Arnfjörð Bjarmason
2021-11-12 22:20                           ` [PATCH v2 09/10] object-name.c: don't have GET_OID_ONLY_TO_DIE imply *_QUIETLY Ævar Arnfjörð Bjarmason
2021-11-12 22:20                           ` [PATCH v2 10/10] cat-file: improve --(textconv|filters) disambiguation Ævar Arnfjörð Bjarmason
2021-11-29 19:57                           ` [PATCH v3 00/10] cat-file: better usage UX & error messages Ævar Arnfjörð Bjarmason
2021-11-29 19:57                             ` [PATCH v3 01/10] cat-file tests: test bad usage Ævar Arnfjörð Bjarmason
2021-11-29 19:57                             ` [PATCH v3 02/10] cat-file tests: test messaging on bad objects/paths Ævar Arnfjörð Bjarmason
2021-11-29 19:57                             ` [PATCH v3 03/10] parse-options API: add a usage_msg_optf() Ævar Arnfjörð Bjarmason
2021-11-29 19:57                             ` [PATCH v3 04/10] cat-file docs: fix SYNOPSIS and "-h" output Ævar Arnfjörð Bjarmason
2021-11-29 19:57                             ` [PATCH v3 05/10] cat-file: move "usage" variable to cmd_cat_file() Ævar Arnfjörð Bjarmason
2021-11-29 19:57                             ` [PATCH v3 06/10] cat-file: make --batch-all-objects a CMDMODE Ævar Arnfjörð Bjarmason
2021-11-29 19:57                             ` [PATCH v3 07/10] cat-file: fix remaining usage bugs Ævar Arnfjörð Bjarmason
2021-12-06  1:19                               ` Jiang Xin
2021-11-29 19:57                             ` [PATCH v3 08/10] cat-file: correct and improve usage information Ævar Arnfjörð Bjarmason
2021-11-29 19:57                             ` [PATCH v3 09/10] object-name.c: don't have GET_OID_ONLY_TO_DIE imply *_QUIETLY Ævar Arnfjörð Bjarmason
2021-11-29 19:57                             ` [PATCH v3 10/10] cat-file: use GET_OID_ONLY_TO_DIE in --(textconv|filters) Ævar Arnfjörð Bjarmason
2021-12-08 12:34                             ` [PATCH v4 00/10] cat-file: better usage UX & error messages Ævar Arnfjörð Bjarmason
2021-12-08 12:34                               ` [PATCH v4 01/10] cat-file tests: test bad usage Ævar Arnfjörð Bjarmason
2021-12-08 12:34                               ` [PATCH v4 02/10] cat-file tests: test messaging on bad objects/paths Ævar Arnfjörð Bjarmason
2021-12-08 12:34                               ` [PATCH v4 03/10] parse-options API: add a usage_msg_optf() Ævar Arnfjörð Bjarmason
2021-12-08 12:34                               ` [PATCH v4 04/10] cat-file docs: fix SYNOPSIS and "-h" output Ævar Arnfjörð Bjarmason
2021-12-08 12:34                               ` [PATCH v4 05/10] cat-file: move "usage" variable to cmd_cat_file() Ævar Arnfjörð Bjarmason
2021-12-08 12:34                               ` [PATCH v4 06/10] cat-file: make --batch-all-objects a CMDMODE Ævar Arnfjörð Bjarmason
2021-12-08 12:34                               ` [PATCH v4 07/10] cat-file: fix remaining usage bugs Ævar Arnfjörð Bjarmason
2021-12-20 16:00                                 ` John Cai
2021-12-08 12:34                               ` [PATCH v4 08/10] cat-file: correct and improve usage information Ævar Arnfjörð Bjarmason
2021-12-08 12:34                               ` [PATCH v4 09/10] object-name.c: don't have GET_OID_ONLY_TO_DIE imply *_QUIETLY Ævar Arnfjörð Bjarmason
2021-12-08 12:34                               ` [PATCH v4 10/10] cat-file: use GET_OID_ONLY_TO_DIE in --(textconv|filters) Ævar Arnfjörð Bjarmason
2021-12-22  4:12                               ` [PATCH v5 00/10] cat-file: better usage UX & error messages Ævar Arnfjörð Bjarmason
2021-12-22  4:12                                 ` [PATCH v5 01/10] cat-file tests: test bad usage Ævar Arnfjörð Bjarmason
2021-12-22  4:12                                 ` [PATCH v5 02/10] cat-file tests: test messaging on bad objects/paths Ævar Arnfjörð Bjarmason
2021-12-22  4:12                                 ` [PATCH v5 03/10] parse-options API: add a usage_msg_optf() Ævar Arnfjörð Bjarmason
2021-12-22  4:12                                 ` [PATCH v5 04/10] cat-file docs: fix SYNOPSIS and "-h" output Ævar Arnfjörð Bjarmason
2021-12-22  4:12                                 ` [PATCH v5 05/10] cat-file: move "usage" variable to cmd_cat_file() Ævar Arnfjörð Bjarmason
2021-12-22  4:12                                 ` [PATCH v5 06/10] cat-file: make --batch-all-objects a CMDMODE Ævar Arnfjörð Bjarmason
2021-12-22  4:13                                 ` [PATCH v5 07/10] cat-file: fix remaining usage bugs Ævar Arnfjörð Bjarmason
2021-12-26  0:31                                   ` Junio C Hamano
2021-12-22  4:13                                 ` [PATCH v5 08/10] cat-file: correct and improve usage information Ævar Arnfjörð Bjarmason
2021-12-22  4:13                                 ` [PATCH v5 09/10] object-name.c: don't have GET_OID_ONLY_TO_DIE imply *_QUIETLY Ævar Arnfjörð Bjarmason
2021-12-22  4:13                                 ` [PATCH v5 10/10] cat-file: use GET_OID_ONLY_TO_DIE in --(textconv|filters) Ævar Arnfjörð Bjarmason
2021-12-28 13:28                                 ` [PATCH v6 00/10] cat-file: better usage UX & error messages Ævar Arnfjörð Bjarmason
2021-12-28 13:28                                   ` [PATCH v6 01/10] cat-file tests: test bad usage Ævar Arnfjörð Bjarmason
2021-12-28 13:28                                   ` [PATCH v6 02/10] cat-file tests: test messaging on bad objects/paths Ævar Arnfjörð Bjarmason
2021-12-28 13:28                                   ` [PATCH v6 03/10] parse-options API: add a usage_msg_optf() Ævar Arnfjörð Bjarmason
2021-12-28 13:28                                   ` [PATCH v6 04/10] cat-file docs: fix SYNOPSIS and "-h" output Ævar Arnfjörð Bjarmason
2021-12-28 13:28                                   ` [PATCH v6 05/10] cat-file: move "usage" variable to cmd_cat_file() Ævar Arnfjörð Bjarmason
2021-12-28 13:28                                   ` [PATCH v6 06/10] cat-file: make --batch-all-objects a CMDMODE Ævar Arnfjörð Bjarmason
2021-12-28 13:28                                   ` [PATCH v6 07/10] cat-file: fix remaining usage bugs Ævar Arnfjörð Bjarmason
2021-12-28 13:28                                   ` [PATCH v6 08/10] cat-file: correct and improve usage information Ævar Arnfjörð Bjarmason
2022-01-08  2:58                                     ` Jiang Xin
2022-01-10 22:08                                       ` [PATCH 0/2] fixups for issues in next-merged ab/cat-file Ævar Arnfjörð Bjarmason
2022-01-10 22:08                                         ` [PATCH 1/2] cat-file: don't whitespace-pad "(...)" in SYNOPSIS and usage output Ævar Arnfjörð Bjarmason
2022-01-10 22:08                                         ` [PATCH 2/2] cat-file: s/_/-/ in typo'd usage_msg_optf() message Ævar Arnfjörð Bjarmason
2022-01-10 22:20                                         ` [PATCH 0/2] fixups for issues in next-merged ab/cat-file Junio C Hamano
2022-01-11 15:48                                         ` Taylor Blau
2022-01-12 18:11                                           ` Junio C Hamano
2021-12-28 13:28                                   ` [PATCH v6 09/10] object-name.c: don't have GET_OID_ONLY_TO_DIE imply *_QUIETLY Ævar Arnfjörð Bjarmason
2021-12-28 13:28                                   ` [PATCH v6 10/10] cat-file: use GET_OID_ONLY_TO_DIE in --(textconv|filters) Ævar Arnfjörð Bjarmason
2021-10-05 20:36 ` [PATCH 3/5] cat-file: disable refs/replace with --batch-all-objects Jeff King
2021-10-06 20:33   ` Derrick Stolee
2021-10-07 20:48   ` Junio C Hamano
2021-10-05 20:36 ` [PATCH 4/5] cat-file: split ordered/unordered batch-all-objects callbacks Jeff King
2021-10-05 20:38 ` Jeff King [this message]
2021-10-07 20:56   ` [PATCH 5/5] cat-file: use packed_object_info() for --batch-all-objects Junio C Hamano
2021-10-08  2:35     ` Jeff King
2021-10-06 20:41 ` [PATCH 0/5] cat-file replace handling and optimization Derrick Stolee
2021-10-07  0:32   ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YVy3rPuUal0+9iJs@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).