[PATCH RFC 0/3] cat-file: make remote-object-info request in batches

[PATCH RFC 0/3] cat-file: make remote-object-info request in batches

[Pablo Sabater]

Currently the command remote-object-info of git cat-file has a hard
limit of 10k OIDs to query in the same command. So, if you want to query
100k objects for example you would have to run the command 10 times.

This seemed strange to me because git backfill for example has a
default limit of 50k objects and backfill fetches full objects, while
remote-object-info only requests metadata (size, type, both or none) so
it should be able to handle at least the same, if not more.

Even if we kept the 10k OID limit for remote-object-info, it is annoying
having to run the command multiple times, so I propose to allow as many
OIDs as wanted and batch them internally.

Regarding which number of objects should be the limit, I benchmarked
different batch sizes over a simulated 47ms RTT (details in patch 2):

+------------+---------------------+
|            | objects requested   |
| batch size |   100k   |   400k   |
+------------+----------+----------+
| 10k        |  1.691s  |  5.862s  |
| 50k        |  0.981s  |  2.397s  |
| 100k       |  0.915s  |  2.058s  |
| 200k       |  0.924s  |  1.889s  |
| 400k       |  0.908s  |  1.820s  |
+------------+----------+----------+

A noticeable difference can be seen from a 10k OID batch to a 50k batch.
After that the difference isn't as noticeable (the number of batches to
be requested isn't that big).

I propose 50k OIDs as the default number of OIDs for the
remote-object-info batches. Coincidentally, git backfill's default batch
size is 50k as well though it's unrelated.

I have barely done any benchmarking before, so I would appreciate
any feedback on the one in patch 2. Is there a better way to do it,
or other things I should take into account?

Should the server also check for a limit on the number of OIDs it
accepts in a single object-info request?

GitHub CI passed:
https://github.com/pabloosabaterr/git/actions/runs/40424912154

Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
---
Pablo Sabater (3):
      cat-file: extract the transport ownership from get_remote_info()
      cat-file: make remote-object-info request in batches
      cat-file: add --remote-max-batch option

 Documentation/git-cat-file.adoc        |  16 ++-
 builtin/cat-file.c                     | 174 +++++++++++++++++++--------------
 t/t1017-cat-file-remote-object-info.sh |  68 +++++++++++++
 transport-helper.c                     |  13 ++-
 transport-internal.h                   |   6 +-
 transport.c                            |  33 +++----
 transport.h                            |   6 +-
 7 files changed, 208 insertions(+), 108 deletions(-)


---
base-commit: 37b3475fb530c14109211faca33716a173db61ed
change-id: 20261015-remote-object-info-batch-c65f13fb5a95

[PATCH RFC 1/3] cat-file: extract the transport ownership from get_remote_info()

[Pablo Sabater]

get_remote_info() receives the name of the remote and from there it
builds the transport and at the end closes it.

To be able to reuse the connection for multiple batches in a subsequent
commit, it is needed to extract that responsibility from
get_remote_info() and instead have it in its caller,
parse_cmd_remote_object_info(), which will handle the batches.

While at it, rename the transport variable 'gtransport' to 'transport',
as it looks like a global when it's not.

get_remote_info() returns -1 when there are no smart options, or
otherwise the result of transport_fetch_object_info(), and its caller
dies if the result is non-zero. Move the smart options check to its
caller and die directly in get_remote_info() if
transport_fetch_object_info() fails, so get_remote_info() can return
void.

At the same time that the smart options check is moved, give it its own
error message, since "failed to get object info from the remote" used
to cover both that case and a failure while requesting the object info.

Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
---
 builtin/cat-file.c | 61 +++++++++++++++++++++++++-----------------------------
 1 file changed, 28 insertions(+), 33 deletions(-)

diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 8870a210ec..f7d75a026b 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -680,55 +680,36 @@ static void batch_one_object(const char *obj_name,
 	object_context_release(&ctx);
 }
 
-static int get_remote_info(int argc,
-			   const char **argv,
-			   struct fetch_object_info_results *results,
-			   struct oid_array *object_info_oids)
+static void get_remote_info(struct transport *transport,
+			    const char **objects,
+			    size_t nr,
+			    struct fetch_object_info_results *results,
+			    struct oid_array *object_info_oids)
 {
-	int retval = 0;
-	struct remote *remote = NULL;
 	struct object_id oid;
-	struct transport *gtransport;
-
-	remote = remote_get(argv[0]);
-	if (!remote)
-		die(_("must supply valid remote when using remote-object-info"));
 
 	oid_array_clear(object_info_oids);
-	for (size_t i = 1; i < argc; i++) {
-		if (get_oid_hex(argv[i], &oid)) {
-			size_t len = strlen(argv[i]);
+	for (size_t i = 0; i < nr; i++) {
+		if (get_oid_hex(objects[i], &oid)) {
+			size_t len = strlen(objects[i]);
 
 			if (len < the_hash_algo->hexsz && len >= 4) {
 				size_t j;
 				for (j = 0; j < len; j++)
-					if (!isxdigit(argv[i][j]))
+					if (!isxdigit(objects[i][j]))
 						break;
 				if (j == len)
 					die(_("remote-object-info does not support "
 					      "short oids, %d characters required"),
 					    (int)the_hash_algo->hexsz);
 			}
-			die(_("not a valid object name '%s'"), argv[i]);
+			die(_("not a valid object name '%s'"), objects[i]);
 		}
 		oid_array_append(object_info_oids, &oid);
 	}
 
-	if (!object_info_oids->nr)
-		die(_("remote-object-info requires objects"));
-
-	gtransport = transport_get(remote, NULL);
-
-	if (!gtransport->smart_options) {
-		retval = -1;
-		goto cleanup;
-	}
-
-	retval = transport_fetch_object_info(gtransport, object_info_oids,
-					     results);
-cleanup:
-	transport_disconnect(gtransport);
-	return retval;
+	if (transport_fetch_object_info(transport, object_info_oids, results))
+		die(_("failed to get object info from the remote"));
 }
 
 struct object_cb_data {
@@ -824,6 +805,8 @@ static void parse_cmd_remote_object_info(struct batch_options *opt,
 	char *line_to_split;
 	struct fetch_object_info_results results = FETCH_OBJECT_INFO_RESULTS_INIT;
 	struct oid_array object_info_oids = OID_ARRAY_INIT;
+	struct transport *transport;
+	struct remote *remote;
 
 	if (strlen(line) >= MAX_REMOTE_OBJ_INFO_LINE)
 		die(_("remote-object-info command too long"));
@@ -837,13 +820,23 @@ static void parse_cmd_remote_object_info(struct batch_options *opt,
 		die(_("remote-object-info supports at most %d objects"),
 		    MAX_ALLOWED_OBJ_LIMIT);
 
+	if (count < 2)
+		die(_("remote-object-info requires objects"));
+
 	if (data->info.sizep)
 		results.wants_size = 1;
 	if (data->info.typep)
 		results.wants_type = 1;
 
-	if (get_remote_info(count, argv, &results, &object_info_oids))
-		die(_("failed to get object info from the remote: %s"), argv[0]);
+	remote = remote_get(argv[0]);
+	if (!remote)
+		die(_("must supply valid remote when using remote-object-info"));
+
+	transport = transport_get(remote, NULL);
+
+	if (!transport->smart_options)
+		die(_("smart options required"));
+	get_remote_info(transport, argv + 1, count - 1, &results, &object_info_oids);
 
 	string_list_clear(&data->remote_allowed_atoms, 0);
 	string_list_append(&data->remote_allowed_atoms, "objectname");
@@ -852,6 +845,8 @@ static void parse_cmd_remote_object_info(struct batch_options *opt,
 	if (results.types)
 		string_list_append(&data->remote_allowed_atoms, "objecttype");
 
+	transport_disconnect(transport);
+
 	data->skip_object_info = 1;
 	for (size_t i = 0; i < results.nr; i++) {
 		data->oid = object_info_oids.oid[i];

-- 
2.56.0

[PATCH RFC 2/3] cat-file: make remote-object-info request in batches

[Pablo Sabater]

remote-object-info has a hard limit of a maximum of 10k OIDs per
command, this means that if someone wants to request the object-info
from +10k objects the user is the one who has to batch it.

Make it more comfortable by allowing a user to put all the OIDs in a
single remote-object-info command and then internally, let
parse_cmd_remote_object_info() handle the batching.

A default max batch size has been set to 50k OIDs. This number comes
from benchmarking with hyperfine against a local git daemon serving a
bare clone of git.git, requesting 100k and 400k OIDs taken from
`rev-list --objects --all` with "%(objectname) %(objectsize)". A 47ms
delay (roughly my connect time to github.com) was added to
client-to-server traffic on port 9418 with dummynet, giving an RTT of
about 47ms:

+------------+---------------------+
|            | objects requested   |
| batch size |   100k   |   400k   |
+------------+----------+----------+
| 10k        |  1.691s  |  5.862s  |
| 50k        |  0.981s  |  2.397s  |
| 100k       |  0.915s  |  2.058s  |
| 200k       |  0.924s  |  1.889s  |
| 400k       |  0.908s  |  1.820s  |
+------------+----------+----------+

To request all the batches over the same connection, it must stay open
between them. Currently, the connection to the remote is closed at two
places:
1. fetch_object_info_via_pack() at transport.c: finish_connect() is
   called after fetching the object-info data.
2. parse_cmd_remote_object_info() at cat-file.c: transport_disconnect()
   is called right after get_remote_info().

About 1, make fetch_object_info_via_pack() perform the handshake only
once and stop closing the connection, leaving that to
transport_disconnect() on its callers.

About 2, move transport_disconnect() after the batching loop, so it
only disconnects from the remote after all the batches have been
requested.

fetch_object_info() is void, so the only failure that
fetch_object_info_via_pack() could report came from finish_connect().
The exit status of the remote process is no longer checked. Any failure
while serving a batch is already caught by fetch_object_info(), either
through an ERR packet or an unexpected EOF, before any output for that
batch is written. This matches how ls-remote treats the connection
after ls-refs.

Make fetch_object_info_via_pack(), fetch_object_info_helper() and
transport_fetch_object_info() return void.

Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
---
 builtin/cat-file.c   | 87 ++++++++++++++++++++++++++--------------------------
 transport-helper.c   | 13 ++++----
 transport-internal.h |  6 ++--
 transport.c          | 33 +++++++++-----------
 transport.h          |  6 ++--
 5 files changed, 69 insertions(+), 76 deletions(-)

diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index f7d75a026b..4f78194bbd 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -39,12 +39,7 @@
  */
 #define MAX_REMOTE_URL_LEN (8 * 1024)
 
-/* Maximum number of objects allowed in a single remote-object-info request. */
-#define MAX_ALLOWED_OBJ_LIMIT 10000
-
-/* Maximum input size permitted for the remote-object-info command. */
-#define MAX_REMOTE_OBJ_INFO_LINE \
-	(MAX_REMOTE_URL_LEN + MAX_ALLOWED_OBJ_LIMIT * (GIT_MAX_HEXSZ + 1))
+#define DEFAULT_REMOTE_BATCH_SIZE 50000
 
 enum batch_mode {
 	BATCH_MODE_CONTENTS,
@@ -708,8 +703,7 @@ static void get_remote_info(struct transport *transport,
 		oid_array_append(object_info_oids, &oid);
 	}
 
-	if (transport_fetch_object_info(transport, object_info_oids, results))
-		die(_("failed to get object info from the remote"));
+	transport_fetch_object_info(transport, object_info_oids, results);
 }
 
 struct object_cb_data {
@@ -805,20 +799,15 @@ static void parse_cmd_remote_object_info(struct batch_options *opt,
 	char *line_to_split;
 	struct fetch_object_info_results results = FETCH_OBJECT_INFO_RESULTS_INIT;
 	struct oid_array object_info_oids = OID_ARRAY_INIT;
+	size_t batch_size = DEFAULT_REMOTE_BATCH_SIZE;
 	struct transport *transport;
 	struct remote *remote;
 
-	if (strlen(line) >= MAX_REMOTE_OBJ_INFO_LINE)
-		die(_("remote-object-info command too long"));
-
 	line_to_split = xstrdup(line);
 	count = split_cmdline(line_to_split, &argv);
 	if (count < 0)
 		die(_("remote-object-info: failed to parse command line: %s"),
 		    split_cmdline_strerror(count));
-	if (count - 1 > MAX_ALLOWED_OBJ_LIMIT)
-		die(_("remote-object-info supports at most %d objects"),
-		    MAX_ALLOWED_OBJ_LIMIT);
 
 	if (count < 2)
 		die(_("remote-object-info requires objects"));
@@ -836,48 +825,58 @@ static void parse_cmd_remote_object_info(struct batch_options *opt,
 
 	if (!transport->smart_options)
 		die(_("smart options required"));
-	get_remote_info(transport, argv + 1, count - 1, &results, &object_info_oids);
 
-	string_list_clear(&data->remote_allowed_atoms, 0);
-	string_list_append(&data->remote_allowed_atoms, "objectname");
-	if (results.sizes)
-		string_list_append(&data->remote_allowed_atoms, "objectsize");
-	if (results.types)
-		string_list_append(&data->remote_allowed_atoms, "objecttype");
+	for (size_t j = 0; j < count - 1; j += batch_size) {
+		const char **objects = argv + 1 + j;
+		size_t remaining = count - 1 - j;
+		size_t objects_nr = batch_size < remaining ? batch_size : remaining;
 
-	transport_disconnect(transport);
+		get_remote_info(transport, objects, objects_nr, &results, &object_info_oids);
 
-	data->skip_object_info = 1;
-	for (size_t i = 0; i < results.nr; i++) {
-		data->oid = object_info_oids.oid[i];
+		string_list_clear(&data->remote_allowed_atoms, 0);
+		string_list_append(&data->remote_allowed_atoms, "objectname");
+		if (results.sizes)
+			string_list_append(&data->remote_allowed_atoms, "objectsize");
+		if (results.types)
+			string_list_append(&data->remote_allowed_atoms, "objecttype");
 
-		if (results.unrecognized[i]) {
-			report_object_status(opt, oid_to_hex(&data->oid),
-					     &data->oid, "missing");
-			continue;
-		}
+		data->skip_object_info = 1;
+		for (size_t i = 0; i < results.nr; i++) {
+			data->oid = object_info_oids.oid[i];
 
-		/*
-		 * When reaching here, it means remote-object-info can retrieve
-		 * information from server without downloading them.
-		 */
-		if (results.sizes)
-			data->size = results.sizes[i];
+			if (results.unrecognized[i]) {
+				report_object_status(opt, oid_to_hex(&data->oid),
+						     &data->oid, "missing");
+				continue;
+			}
 
-		if (results.types)
-			data->type = results.types[i];
+			/*
+			 * When reaching here, it means remote-object-info can retrieve
+			 * information from server without downloading them.
+			 */
+			if (results.sizes)
+				data->size = results.sizes[i];
 
-		opt->batch_mode = BATCH_MODE_INFO;
-		data->is_remote = 1;
-		batch_object_write(argv[i + 1], output, opt, data, NULL, 0);
-		data->is_remote = 0;
+			if (results.types)
+				data->type = results.types[i];
+
+			opt->batch_mode = BATCH_MODE_INFO;
+			data->is_remote = 1;
+			batch_object_write(objects[i], output, opt, data, NULL, 0);
+			data->is_remote = 0;
+		}
+		data->skip_object_info = 0;
+		FREE_AND_NULL(results.sizes);
+		FREE_AND_NULL(results.types);
+		FREE_AND_NULL(results.unrecognized);
+		results.nr = 0;
+		oid_array_clear(&object_info_oids);
 	}
-	data->skip_object_info = 0;
 
+	transport_disconnect(transport);
 	free_fetch_object_info_results(&results);
 	free(line_to_split);
 	free(argv);
-	oid_array_clear(&object_info_oids);
 }
 
 static void dispatch_calls(struct batch_options *opt,
diff --git a/transport-helper.c b/transport-helper.c
index d5a064d386..dafe829b50 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -786,16 +786,15 @@ static int fetch_refs(struct transport *transport,
 	return -1;
 }
 
-static int fetch_object_info_helper(struct transport *transport,
-				    const struct oid_array *oids,
-				    struct fetch_object_info_results *results)
+static void fetch_object_info_helper(struct transport *transport,
+				     const struct oid_array *oids,
+				     struct fetch_object_info_results *results)
 {
 	get_helper(transport);
 	if (process_connect(transport, 0))
-		return transport->vtable->fetch_object_info(transport, oids,
-							    results);
-
-	die(_("object-info requires protocol v2"));
+		transport->vtable->fetch_object_info(transport, oids, results);
+	else
+		die(_("object-info requires protocol v2"));
 }
 
 struct push_update_ref_state {
diff --git a/transport-internal.h b/transport-internal.h
index a10b27cc81..53ce86ed48 100644
--- a/transport-internal.h
+++ b/transport-internal.h
@@ -53,9 +53,9 @@ struct transport_vtable {
 	 *
 	 * Uses object-info capability of v2 protocol.
 	 */
-	int (*fetch_object_info)(struct transport *transport,
-				 const struct oid_array *oids,
-				 struct fetch_object_info_results *results);
+	void (*fetch_object_info)(struct transport *transport,
+				  const struct oid_array *oids,
+				  struct fetch_object_info_results *results);
 
 	/**
 	 * Push the objects and refs. Send the necessary objects, and
diff --git a/transport.c b/transport.c
index 25e2c14a7b..352540492c 100644
--- a/transport.c
+++ b/transport.c
@@ -433,11 +433,10 @@ static int get_bundle_uri(struct transport *transport)
 				     transport->bundles, stateless_rpc);
 }
 
-static int fetch_object_info_via_pack(struct transport *transport,
-				      const struct oid_array *oids,
-				      struct fetch_object_info_results *results)
+static void fetch_object_info_via_pack(struct transport *transport,
+				       const struct oid_array *oids,
+				       struct fetch_object_info_results *results)
 {
-	int ret = 0;
 	struct git_transport_data *data = transport->data;
 	struct packet_reader reader;
 
@@ -447,8 +446,13 @@ static int fetch_object_info_via_pack(struct transport *transport,
 			   PACKET_READ_GENTLE_ON_EOF |
 			   PACKET_READ_DIE_ON_ERR_PACKET);
 
-	data->version = discover_version(&reader);
-	transport->hash_algo = reader.hash_algo;
+	if (!data->finished_handshake) {
+		data->version = discover_version(&reader);
+		transport->hash_algo = reader.hash_algo;
+		data->finished_handshake = 1;
+	} else {
+		reader.hash_algo = transport->hash_algo;
+	}
 
 	fetch_object_info(data->version,
 			  transport->server_options,
@@ -456,24 +460,15 @@ static int fetch_object_info_via_pack(struct transport *transport,
 			  &reader,
 			  results,
 			  transport->stateless_rpc, data->fd[1]);
-
-	close(data->fd[0]);
-	if (data->fd[1] >= 0)
-		close(data->fd[1]);
-	if (finish_connect(data->conn))
-		ret = -1;
-	data->conn = NULL;
-
-	return ret;
 }
 
-int transport_fetch_object_info(struct transport *transport,
-				const struct oid_array *oids,
-				struct fetch_object_info_results *results)
+void transport_fetch_object_info(struct transport *transport,
+				 const struct oid_array *oids,
+				 struct fetch_object_info_results *results)
 {
 	if (!transport->vtable->fetch_object_info)
 		die(_("remote does not support object-info"));
-	return transport->vtable->fetch_object_info(transport, oids, results);
+	transport->vtable->fetch_object_info(transport, oids, results);
 }
 
 static int fetch_refs_via_pack(struct transport *transport,
diff --git a/transport.h b/transport.h
index 39193d0077..c7f2b8b87f 100644
--- a/transport.h
+++ b/transport.h
@@ -314,9 +314,9 @@ int transport_fetch_refs(struct transport *transport, struct ref *refs);
 /*
  * Fetch the object info from remote
  */
-int transport_fetch_object_info(struct transport *transport,
-				const struct oid_array *oids,
-				struct fetch_object_info_results *results);
+void transport_fetch_object_info(struct transport *transport,
+				 const struct oid_array *oids,
+				 struct fetch_object_info_results *results);
 
 /*
  * If this flag is set, unlocking will avoid to call non-async-signal-safe

-- 
2.56.0

[PATCH RFC 3/3] cat-file: add --remote-max-batch option

[Pablo Sabater]

Now that remote-object-info works by requesting in batches, a user might
not want to use the default of 50k OIDs per batch.

Add a new --remote-max-batch option that changes the size of the
batches.

Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
---
 Documentation/git-cat-file.adoc        | 16 ++++++--
 builtin/cat-file.c                     | 46 ++++++++++++++++++++---
 t/t1017-cat-file-remote-object-info.sh | 68 ++++++++++++++++++++++++++++++++++
 3 files changed, 121 insertions(+), 9 deletions(-)

diff --git a/Documentation/git-cat-file.adoc b/Documentation/git-cat-file.adoc
index c4ea2524cf..a21a4d7fd4 100644
--- a/Documentation/git-cat-file.adoc
+++ b/Documentation/git-cat-file.adoc
@@ -14,7 +14,7 @@ SYNOPSIS
 	     [<rev>:<path|tree-ish> | --path=<path|tree-ish> <rev>]
 'git cat-file' (--batch | --batch-check | --batch-command) [--batch-all-objects]
 	     [--buffer] [--follow-symlinks] [--unordered]
-	     [--textconv | --filters] [-Z]
+	     [--textconv | --filters] [-Z] [--remote-max-batch=<n>]
 
 DESCRIPTION
 -----------
@@ -143,8 +143,8 @@ are not of the requested type.
 --batch-command::
 --batch-command=<format>::
 	Enter a command mode that reads commands and arguments from stdin. May
-	only be combined with `--buffer`, `--textconv`, `--use-mailmap` or
-	`--filters`.
+	only be combined with `--buffer`, `--textconv`, `--use-mailmap`,
+	`--filters` or `--remote-max-batch`.
 +
 --
 	* When used with `--textconv` or `--filters`, the input lines must
@@ -174,6 +174,9 @@ remote-object-info <remote> <object>...::
 	`<remote>` without downloading objects from the remote.
 	Raise an error when the `object-info` capability is not supported by the remote.
 	Raise an error when no object references are provided.
+	There is no limit on the number of objects; they are requested from
+	the remote in batches over a single connection, see
+	`--remote-max-batch`.
 	This command may be combined with `--buffer`.
 
 flush::
@@ -215,6 +218,13 @@ flush::
 	only once, even if it is stored multiple times in the
 	repository.
 
+--remote-max-batch=<n>::
+	Maximum number of objects that `remote-object-info` asks for in a
+	single `object-info` request. If a `remote-object-info` command is
+	given more objects, the rest are requested in further batches over
+	the same connection. `<n>` must be a positive integer. Defaults
+	to 50000. Requires `--batch-command`.
+
 --follow-symlinks::
 	With `--batch` or `--batch-check`, follow symlinks inside the
 	repository when requesting objects with extended SHA-1
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 4f78194bbd..ca2122c67b 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -59,6 +59,8 @@ struct batch_options {
 	char input_delim;
 	char output_delim;
 	const char *format;
+	size_t remote_max_batch;
+	unsigned custom_object_info_batch;
 };
 
 static const char *force_path;
@@ -799,9 +801,9 @@ static void parse_cmd_remote_object_info(struct batch_options *opt,
 	char *line_to_split;
 	struct fetch_object_info_results results = FETCH_OBJECT_INFO_RESULTS_INIT;
 	struct oid_array object_info_oids = OID_ARRAY_INIT;
-	size_t batch_size = DEFAULT_REMOTE_BATCH_SIZE;
 	struct transport *transport;
 	struct remote *remote;
+	size_t object_nr;
 
 	line_to_split = xstrdup(line);
 	count = split_cmdline(line_to_split, &argv);
@@ -811,6 +813,7 @@ static void parse_cmd_remote_object_info(struct batch_options *opt,
 
 	if (count < 2)
 		die(_("remote-object-info requires objects"));
+	object_nr = (size_t)count - 1;
 
 	if (data->info.sizep)
 		results.wants_size = 1;
@@ -826,12 +829,16 @@ static void parse_cmd_remote_object_info(struct batch_options *opt,
 	if (!transport->smart_options)
 		die(_("smart options required"));
 
-	for (size_t j = 0; j < count - 1; j += batch_size) {
+	for (size_t j = 0; j < object_nr; j += opt->remote_max_batch) {
 		const char **objects = argv + 1 + j;
-		size_t remaining = count - 1 - j;
-		size_t objects_nr = batch_size < remaining ? batch_size : remaining;
+		size_t objects_in_batch_nr;
 
-		get_remote_info(transport, objects, objects_nr, &results, &object_info_oids);
+		if (opt->remote_max_batch < object_nr - j)
+			objects_in_batch_nr = opt->remote_max_batch;
+		else
+			objects_in_batch_nr = object_nr - j;
+
+		get_remote_info(transport, objects, objects_in_batch_nr, &results, &object_info_oids);
 
 		string_list_clear(&data->remote_allowed_atoms, 0);
 		string_list_append(&data->remote_allowed_atoms, "objectname");
@@ -1145,6 +1152,24 @@ static int git_cat_file_config(const char *var, const char *value,
 	return git_default_config(var, value, ctx, cb);
 }
 
+static int set_remote_batch_size(const struct option *opt,
+				 const char *arg,
+				 int unset)
+{
+	struct batch_options *bo = opt->value;
+	size_t n;
+
+	BUG_ON_OPT_NEG(unset);
+
+	if (!git_parse_size_t(arg, &n) || !n)
+		return error(_("%s expects a positive number"), opt->long_name);
+
+	bo->remote_max_batch = n;
+	bo->custom_object_info_batch = 1;
+
+	return 0;
+}
+
 static int batch_option_callback(const struct option *opt,
 				 const char *arg,
 				 int unset)
@@ -1184,6 +1209,7 @@ int cmd_cat_file(int argc,
 	const char *exp_type = NULL, *obj_name = NULL;
 	struct batch_options batch = {
 		.objects_filter = LIST_OBJECTS_FILTER_INIT,
+		.remote_max_batch = DEFAULT_REMOTE_BATCH_SIZE,
 	};
 	int unknown_type = 0;
 	int input_nul_terminated = 0;
@@ -1197,7 +1223,7 @@ int cmd_cat_file(int argc,
 		   "             [<rev>:<path|tree-ish> | --path=<path|tree-ish> <rev>]"),
 		N_("git cat-file (--batch | --batch-check | --batch-command) [--batch-all-objects]\n"
 		   "             [--buffer] [--follow-symlinks] [--unordered]\n"
-		   "             [--textconv | --filters] [-Z]"),
+		   "             [--textconv | --filters] [-Z] [--remote-max-batch=<n>]"),
 		NULL
 	};
 	const struct option options[] = {
@@ -1240,6 +1266,9 @@ int cmd_cat_file(int argc,
 			 N_("follow in-tree symlinks")),
 		OPT_BOOL(0, "unordered", &batch.unordered,
 			 N_("do not order objects before emitting them")),
+		OPT_CALLBACK_F(0, "remote-max-batch", &batch, N_("n"),
+			       N_("set a custom object-info batch size"),
+			       PARSE_OPT_NONEG, set_remote_batch_size),
 		/* Textconv options, stand-ole*/
 		OPT_GROUP(N_("Emit object (blob or tree) with conversion or filter (stand-alone, or with batch)")),
 		OPT_CMDMODE(0, "textconv", &opt,
@@ -1307,6 +1336,11 @@ int cmd_cat_file(int argc,
 		usage_msg_optf(_("'%s' requires a batch mode"), builtin_catfile_usage,
 			       options, "-Z");
 
+	if (batch.custom_object_info_batch &&
+	    batch.batch_mode != BATCH_MODE_QUEUE_AND_DISPATCH)
+		usage_msg_optf(_("'%s' requires '%s'"), builtin_catfile_usage,
+			       options, "--remote-max-batch", "--batch-command");
+
 	batch.input_delim = batch.output_delim = '\n';
 	if (input_nul_terminated)
 		batch.input_delim = '\0';
diff --git a/t/t1017-cat-file-remote-object-info.sh b/t/t1017-cat-file-remote-object-info.sh
index e2919aa061..75624a9498 100755
--- a/t/t1017-cat-file-remote-object-info.sh
+++ b/t/t1017-cat-file-remote-object-info.sh
@@ -61,6 +61,18 @@ set_transport_variables () {
 # Since "%(objecttype)" is currently not supported by the command remote-object-info ,
 # the filters are set to "%(objectname) %(objectsize)" in some test cases.
 
+test_expect_success '--remote-max-batch rejects zero and non-numbers' '
+	test_must_fail git cat-file --batch-command --remote-max-batch=0 </dev/null 2>err &&
+	test_grep "remote-max-batch expects a positive number" err &&
+	test_must_fail git cat-file --batch-command --remote-max-batch=foo </dev/null 2>err &&
+	test_grep "remote-max-batch expects a positive number" err
+'
+
+test_expect_success '--remote-max-batch requires --batch-command' '
+	test_must_fail git cat-file --batch-check --remote-max-batch=2 </dev/null 2>err &&
+	test_grep "requires .--batch-command." err
+'
+
 # Test --batch-command remote-object-info with 'git://' transport with
 # transfer.advertiseobjectinfo set to true, i.e. server has object-info capability
 . "$TEST_DIRECTORY"/lib-git-daemon.sh
@@ -134,6 +146,25 @@ test_expect_success 'batch-command remote-object-info git:// multiple sha1 per l
 	)
 '
 
+test_expect_success 'remote-object-info git:// across batch boundaries' '
+	(
+		set_transport_variables "$daemon_parent" &&
+		cd "$daemon_parent/daemon_client_empty" &&
+
+		echo "$hello_oid $hello_size" >expect &&
+		echo "$tree_oid $tree_size" >>expect &&
+		echo "$unstored_oid missing" >>expect &&
+		echo "$commit_oid $commit_size" >>expect &&
+		echo "$tag_oid $tag_size" >>expect &&
+
+		git cat-file --batch-command="%(objectname) %(objectsize)" \
+			--remote-max-batch=2 >actual <<-EOF &&
+		remote-object-info "$GIT_DAEMON_URL/parent" $hello_oid $tree_oid $unstored_oid $commit_oid $tag_oid
+		EOF
+		test_cmp expect actual
+	)
+'
+
 test_expect_success 'batch-command remote-object-info git:// default filter' '
 	(
 		set_transport_variables "$daemon_parent" &&
@@ -461,6 +492,25 @@ test_expect_success 'batch-command remote-object-info file:// default filter' '
 	)
 '
 
+test_expect_success 'remote-object-info file:// with objects a multiple of the batch size' '
+	(
+		set_transport_variables "server" &&
+		server_path="$(pwd)/server" &&
+		cd file_client_empty &&
+
+		echo "$hello_oid $hello_size" >expect &&
+		echo "$tree_oid $tree_size" >>expect &&
+		echo "$commit_oid $commit_size" >>expect &&
+		echo "$tag_oid $tag_size" >>expect &&
+
+		git cat-file --batch-command="%(objectname) %(objectsize)" \
+			--remote-max-batch=2 >actual <<-EOF &&
+		remote-object-info "file://${server_path}" $hello_oid $tree_oid $commit_oid $tag_oid
+		EOF
+		test_cmp expect actual
+	)
+'
+
 test_expect_success 'batch-command -Z remote-object-info file:// default filter' '
 	(
 		set_transport_variables "server" &&
@@ -741,6 +791,24 @@ test_expect_success 'remote-object-info fails on not providing OID' '
 	)
 '
 
+test_expect_success 'remote-object-info http:// across batch boundaries' '
+	(
+		set_transport_variables "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" &&
+		cd "$HTTPD_DOCUMENT_ROOT_PATH/http_client_empty" &&
+
+		echo "$hello_oid $hello_size" >expect &&
+		echo "$tree_oid $tree_size" >>expect &&
+		echo "$unstored_oid missing" >>expect &&
+		echo "$commit_oid $commit_size" >>expect &&
+		echo "$tag_oid $tag_size" >>expect &&
+
+		git cat-file --batch-command="%(objectname) %(objectsize)" \
+			--remote-max-batch=2 >actual <<-EOF &&
+		remote-object-info "$HTTPD_URL/smart/http_parent" $hello_oid $tree_oid $unstored_oid $commit_oid $tag_oid
+		EOF
+		test_cmp expect actual
+	)
+'
 
 # Test --batch-command remote-object-info with 'http://' transport and
 # transfer.advertiseobjectinfo set to false, i.e. server does not have object-info capability

-- 
2.56.0

Re: [PATCH RFC 1/3] cat-file: extract the transport ownership from get_remote_info()

[Junio C Hamano]

Pablo Sabater <pabloosabaterr@gmail.com> writes:

> get_remote_info() receives the name of the remote and from there it
> builds the transport and at the end closes it.
>
> To be able to reuse the connection for multiple batches in a subsequent
> commit, it is needed to extract that responsibility from
> get_remote_info() and instead have it in its caller,
> parse_cmd_remote_object_info(), which will handle the batches.

Makes sense.

> While at it, rename the transport variable 'gtransport' to 'transport',
> as it looks like a global when it's not.

Good move.  We started (not particularly this program but the
transport layer in general) with a global instance of transport
object called gtransport, but the ones that we create on demand and
release after we are done are of course local and it is misleading
to use such a prefix.

> +	if (!transport->smart_options)
> +		die(_("smart options required"));

Is this an end-user facing error message?  I am not sure a user who
saw this message can figure out what went wrong or how to correct
it.

Re: [PATCH RFC 0/3] cat-file: make remote-object-info request in batches

[Junio C Hamano]

Pablo Sabater <pabloosabaterr@gmail.com> writes:

> A noticeable difference can be seen from a 10k OID batch to a 50k batch.
> After that the difference isn't as noticeable (the number of batches to
> be requested isn't that big).
>
> I propose 50k OIDs as the default number of OIDs for the
> remote-object-info batches. Coincidentally, git backfill's default batch
> size is 50k as well though it's unrelated.

Interesting.  We cannot share any code with backfill as we only
retrieve info on objects while backfill does grab the objects
themselves, but using the same number may make it easy to explain in
the documentation.

> I have barely done any benchmarking before, so I would appreciate
> any feedback on the one in patch 2. Is there a better way to do it,
> or other things I should take into account?
>
> Should the server also check for a limit on the number of OIDs it
> accepts in a single object-info request?

What I am more curious about is how this updated code interacts with
existing code (i.e., updated "cat-file --batch" clients with deployed
servers, and vice versa).

Re: [PATCH RFC 0/3] cat-file: make remote-object-info request in batches

[Pablo Sabater]

On Tue Oct 27, 2026 at 7:12 PM WET, Junio C Hamano wrote:
> Pablo Sabater <pabloosabaterr@gmail.com> writes:
>
>> A noticeable difference can be seen from a 10k OID batch to a 50k batch.
>> After that the difference isn't as noticeable (the number of batches to
>> be requested isn't that big).
>>
>> I propose 50k OIDs as the default number of OIDs for the
>> remote-object-info batches. Coincidentally, git backfill's default batch
>> size is 50k as well though it's unrelated.
>
> Interesting.  We cannot share any code with backfill as we only
> retrieve info on objects while backfill does grab the objects
> themselves, but using the same number may make it easy to explain in
> the documentation.
>
>> I have barely done any benchmarking before, so I would appreciate
>> any feedback on the one in patch 2. Is there a better way to do it,
>> or other things I should take into account?
>>
>> Should the server also check for a limit on the number of OIDs it
>> accepts in a single object-info request?
>
> What I am more curious about is how this updated code interacts with
> existing code (i.e., updated "cat-file --batch" clients with deployed
> servers, and vice versa).

The code in this series only changes the client side, the server has
no OID limit. Before this series, what prevented large requests was the
client with its 10k hard limit, so the server is fine with any size
without any change.

The server will see requests of at most 10k OIDs from old clients, and
bigger ones from new clients.

The other change compared to an older client is that now multiple
requests can be received on the same connection (instead of one per
connection), but that should be fine, serve() keeps processing requests
until EOF.

With this new option there's a risk (that was already there if someone
modified their client) of setting a huge amount of OIDs in the same
request, but I'm not sure how bad that is, a 1M OID request vs smaller
ones on the same connection. Could you help me with this?

Anyway, the fix would be to add a cap on the server side. It can't be
lower than 10k, or older clients would get an error they can't do
anything about, and if it's lower than the client's batch size it
should be advertised so newer clients can adapt.

IIRC clients ignore unknown capability keys, so advertising it shouldn't
break older ones. In this case, it would be advertised on the handshake
and the client would die() if the user sets a batch size bigger than
what the server advertises. Ideally the default on the client (the
proposed 50k) would be the min cap on the server, so someone that
doesn't manually set the batch size and uses the default value doesn't
get an error.

If we don't even want to die() we could set the batch size as:
	min(user-size, advertised-size)

Sorry for the brainstorm but I just realised that any client could
modify the size and that a cap on the server side could be important.

Regards,
Pablo

Re: [PATCH RFC 0/3] cat-file: make remote-object-info request in batches

[Junio C Hamano]

"Pablo Sabater" <pabloosabaterr@gmail.com> writes:

> Sorry for the brainstorm but I just realised that any client could
> modify the size and that a cap on the server side could be important.

No need to be sorry.  A major role of review comments is to help us
think how we can improve the system, including but not necessarily
limited to the changes proposed by the reviewed topic.

Re: [PATCH RFC 1/3] cat-file: extract the transport ownership from get_remote_info()

[Pablo Sabater]

On Tue Oct 27, 2026 at 7:02 PM WET, Junio C Hamano wrote:
> Pablo Sabater <pabloosabaterr@gmail.com> writes:
>
>> get_remote_info() receives the name of the remote and from there it
>> builds the transport and at the end closes it.
>>
>> To be able to reuse the connection for multiple batches in a subsequent
>> commit, it is needed to extract that responsibility from
>> get_remote_info() and instead have it in its caller,
>> parse_cmd_remote_object_info(), which will handle the batches.
>
> Makes sense.
>
>> While at it, rename the transport variable 'gtransport' to 'transport',
>> as it looks like a global when it's not.
>
> Good move.  We started (not particularly this program but the
> transport layer in general) with a global instance of transport
> object called gtransport, but the ones that we create on demand and
> release after we are done are of course local and it is misleading
> to use such a prefix.
>
>> +	if (!transport->smart_options)
>> +		die(_("smart options required"));
>
> Is this an end-user facing error message?  I am not sure a user who
> saw this message can figure out what went wrong or how to correct
> it.

I thought it would be better to say why it fails instead of a generic
error. If we drop the check it will die() later with "remote does not
support object-info".

Since the smart options check only die()s when the remote is a bundle,
I could keep it and change the message to "object-info is not supported
on bundles".

Regards,
Pablo

Re: [PATCH RFC 1/3] cat-file: extract the transport ownership from get_remote_info()

[Junio C Hamano]

"Pablo Sabater" <pabloosabaterr@gmail.com> writes:

>>> +	if (!transport->smart_options)
>>> +		die(_("smart options required"));
>>
>> Is this an end-user facing error message?  I am not sure a user who
>> saw this message can figure out what went wrong or how to correct
>> it.
>
> I thought it would be better to say why it fails instead of a generic
> error. If we drop the check it will die() later with "remote does not
> support object-info".
>
> Since the smart options check only die()s when the remote is a bundle,
> I could keep it and change the message to "object-info is not supported
> on bundles".

I am somewhat torn on this.  It is certainly much better than the
"smart options required" message, which the end user would not know
how to act upon.  However, that improvement only holds until we gain
another transport other than bundle that also lacks support for
"smart options".  Saying something like "object-info is not
supported by this remote." would avoid blindly mentioning "bundle"
when the remote that lacks object-info support is actually not a
bundle, but it loses the clarity your rewritten message provides
when the problematic remote indeed is a bundle.

Do we have the remote information at this point in the code (i.e., the
URL of the remote that was given to the remote-object-info command)
that the user would immediately recognize as the remote they asked
the command to access?  If so, "object-info is not supported by %s"
filled with the remote information would probably work better.

Thanks.

Re: [PATCH RFC 1/3] cat-file: extract the transport ownership from get_remote_info()

[Pablo Sabater]

On Wed Oct 28, 2026 at 6:37 AM WET, Junio C Hamano wrote:
> "Pablo Sabater" <pabloosabaterr@gmail.com> writes:
>
>>>> +	if (!transport->smart_options)
>>>> +		die(_("smart options required"));
>>>
>>> Is this an end-user facing error message?  I am not sure a user who
>>> saw this message can figure out what went wrong or how to correct
>>> it.
>>
>> I thought it would be better to say why it fails instead of a generic
>> error. If we drop the check it will die() later with "remote does not
>> support object-info".
>>
>> Since the smart options check only die()s when the remote is a bundle,
>> I could keep it and change the message to "object-info is not supported
>> on bundles".
>
> I am somewhat torn on this.  It is certainly much better than the
> "smart options required" message, which the end user would not know
> how to act upon.  However, that improvement only holds until we gain
> another transport other than bundle that also lacks support for
> "smart options".  Saying something like "object-info is not
> supported by this remote." would avoid blindly mentioning "bundle"
> when the remote that lacks object-info support is actually not a
> bundle, but it loses the clarity your rewritten message provides
> when the problematic remote indeed is a bundle.
>
> Do we have the remote information at this point in the code (i.e., the
> URL of the remote that was given to the remote-object-info command)
> that the user would immediately recognize as the remote they asked
> the command to access?  If so, "object-info is not supported by %s"
> filled with the remote information would probably work better.

We have what the user wrote for the remote in argv[0], so we could have
"object-info is not supported by <remote>", and it would be the same
for anything that sets smart_options to NULL.

But with that in mind, what if in the future there's a transport without
smart_options that does support object-info? The smart_options check is
there because of "this transport can't do object-info", and nothing
on the object-info path reads smart_options from what I checked.

So I'm going to drop that die(). Later in the code there's already a 
check on transport_fetch_object_info(), which looks for 
fetch_object_info in the transport's vtable. It's a bit later, but it 
checks the real capability and it will give less trouble in the future. 

Currently, it has a generic error but it can print a more detailed 
version such as: "object-info is not supported by <remote>"

I'll send v2 later this week as I have to study for some of my midterms.

Regards,
Pablo

>
> Thanks.

Re: [PATCH RFC 1/3] cat-file: extract the transport ownership from get_remote_info()

[Junio C Hamano]

"Pablo Sabater" <pabloosabaterr@gmail.com> writes:

> But with that in mind, what if in the future there's a transport without
> smart_options that does support object-info? The smart_options check is
> there because of "this transport can't do object-info", and nothing
> on the object-info path reads smart_options from what I checked.
>
> So I'm going to drop that die(). Later in the code there's already a 
> check on transport_fetch_object_info(), which looks for 
> fetch_object_info in the transport's vtable. It's a bit later, but it 
> checks the real capability and it will give less trouble in the future. 

That design decision makes a lot of sense.  Thanks.