[PATCH] http-backend: drop strbuf pre-sizing from get_info_packs()

[PATCH] http-backend: drop strbuf pre-sizing from get_info_packs()

[Yusuke Hayashi via B4 Relay]

From: Yusuke Hayashi <yusuke8h@gmail.com>

get_info_packs() iterates over all packs in the repository to count
local packs, calls `strbuf_grow(&buf, cnt * 53 + 2)`, and then iterates
over the packs a second time to format each entry with `strbuf_addf()`.

This pre-sizing dates back to 2f4038ab33 (Git-aware CGI to provide dumb
HTTP transport, 2009-10-30) and is no longer worth keeping: `53` assumes
the length of a SHA-1 "P pack-<hex>.pack\n" line (under-counting the
77-byte lines in SHA-256 repositories), `cnt * 53 + 2` uses unchecked
arithmetic, and `strbuf_addf()` / `strbuf_addch()` already grow the
buffer automatically without needing an extra pass over the pack list.
Similar pre-sizing was recently removed in a9ce8526dc (pretty: drop
strbuf pre-sizing from add_rfc2047(), 2026-05-12).

Drop `cnt`, the counting loop, and the `strbuf_grow()` call.

Signed-off-by: Yusuke Hayashi <yusuke8h@gmail.com>
---
Inspired by https://lore.kernel.org/git/20260513185408.GA147423@coredump.intra.peff.net/
---
 http-backend.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/http-backend.c b/http-backend.c
index c7566b1d12..da9516a4c1 100644
--- a/http-backend.c
+++ b/http-backend.c
@@ -612,15 +612,8 @@ static void get_info_packs(struct strbuf *hdr, char *arg UNUSED)
 	size_t objdirlen = strlen(repo_get_object_directory(the_repository));
 	struct strbuf buf = STRBUF_INIT;
 	struct packed_git *p;
-	size_t cnt = 0;
 
 	select_getanyfile(hdr);
-	repo_for_each_pack(the_repository, p) {
-		if (p->pack_local)
-			cnt++;
-	}
-
-	strbuf_grow(&buf, cnt * 53 + 2);
 	repo_for_each_pack(the_repository, p) {
 		if (p->pack_local)
 			strbuf_addf(&buf, "P %s\n", p->pack_name + objdirlen + 6);

---
base-commit: 9354c5f04055932909b660a237905b360cba195a
change-id: 20261113-http-backend-drop-strbuf-presizing-6f496552d0ca

Re: [PATCH] http-backend: drop strbuf pre-sizing from get_info_packs()

[Patrick Steinhardt]

On Fri, Nov 13, 2026 at 02:17:58AM +0000, Yusuke Hayashi via B4 Relay wrote:
> From: Yusuke Hayashi <yusuke8h@gmail.com>
> 
> get_info_packs() iterates over all packs in the repository to count
> local packs, calls `strbuf_grow(&buf, cnt * 53 + 2)`, and then iterates
> over the packs a second time to format each entry with `strbuf_addf()`.
> 
> This pre-sizing dates back to 2f4038ab33 (Git-aware CGI to provide dumb
> HTTP transport, 2009-10-30) and is no longer worth keeping: `53` assumes
> the length of a SHA-1 "P pack-<hex>.pack\n" line (under-counting the
> 77-byte lines in SHA-256 repositories), `cnt * 53 + 2` uses unchecked
> arithmetic, and `strbuf_addf()` / `strbuf_addch()` already grow the
> buffer automatically without needing an extra pass over the pack list.
> Similar pre-sizing was recently removed in a9ce8526dc (pretty: drop
> strbuf pre-sizing from add_rfc2047(), 2026-05-12).

So this explains that the behaviour is buggy, but that may lead the
reader to wonder why we don't just fix the bug. And sure, we know to
grow the buffer automatically, but the hint is intended as a performance
improvement so that we know to allocate the buffer properly right from
the start.

Note that I'm not saying that this change doesn't make sense. But I
think the argument that "others do it" is rather weak. The more
important question to answer is why it's fine to skip it in this
particular function.

Thanks!

Patrick

Re: [PATCH] http-backend: drop strbuf pre-sizing from get_info_packs()

[Yusuke Hayashi]

On Fri, Nov 13, 2026 at 08:03:52AM +0100, Patrick Steinhardt wrote:
> So this explains that the behaviour is buggy, but that may lead the
> reader to wonder why we don't just fix the bug. And sure, we know to
> grow the buffer automatically, but the hint is intended as a performance
> improvement so that we know to allocate the buffer properly right from
> the start.
>
> Note that I'm not saying that this change doesn't make sense. But I
> think the argument that "others do it" is rather weak. The more
> important question to answer is why it's fine to skip it in this
> particular function.

Fair point -- pointing out the SHA-256 under-count and unchecked
multiplication without explaining why we don't just fix the formula
leaves the main question unanswered, and citing a9ce8526dc doesn't
answer it for get_info_packs() either.

In this case, get_info_packs() is a one-shot CGI handler for dumb HTTP
GET /objects/info/packs requests that formats a single response and
exits. A typical repository only produces a few kilobytes of output, and
even with thousands of packs strbuf's geometric growth only needs a
couple dozen reallocations, which is negligible compared to CGI startup,
scanning objects/pack to populate the pack list, and writing the HTTP
response. Meanwhile, computing the hint up front requires an extra pass
over the pack list.

I'll send a v2 with an updated commit message that explains this
directly instead.

Thanks for the review!

[PATCH v2] http-backend: drop strbuf pre-sizing from get_info_packs()

[Yusuke Hayashi via B4 Relay]

From: Yusuke Hayashi <yusuke8h@gmail.com>

`get_info_packs()` iterates over all packs in the repository to count
local packs, calls `strbuf_grow(&buf, cnt * 53 + 2)`, and then iterates
over the packs a second time to format each entry with `strbuf_addf()`.
This pre-sizing dates back to 2f4038ab33 (Git-aware CGI to provide dumb
HTTP transport, 2009-10-30): `53` assumes the length of a SHA-1
"P pack-<hex>.pack\n" line (under-counting the 77-byte lines in SHA-256
repositories), and `cnt * 53 + 2` uses unchecked arithmetic.

Rather than fixing the size calculation with `the_hash_algo->hexsz` and
`st_mult()` / `st_add()`, drop the pre-sizing altogether.
`get_info_packs()` is a one-shot CGI handler for dumb HTTP
`/objects/info/packs` requests that formats a single response and exits.
A typical repository only has a few dozen packs (a few kilobytes of
output), and even with thousands of packs `strbuf`'s geometric growth
only performs a couple dozen reallocations. That cost is negligible next
to CGI startup, scanning `objects/pack` to populate the pack list, and
writing the HTTP response, whereas computing `cnt` up front requires an
extra pass over the pack list.

Drop `cnt`, the counting loop, and the `strbuf_grow()` call, and let
`strbuf_addf()` and `strbuf_addch()` grow the buffer as needed.

Signed-off-by: Yusuke Hayashi <yusuke8h@gmail.com>
---
Inspired by https://lore.kernel.org/git/20260513185408.GA147423@coredump.intra.peff.net/
---
Changes in v2:
- Rewrite the commit message to explain why skipping pre-sizing is fine
  in `get_info_packs()` specifically (one-shot CGI handler where
  `strbuf`'s geometric growth is negligible next to CGI startup, pack
  discovery, and HTTP I/O), rather than citing `a9ce8526dc` as precedent.
- No code changes.
- Link to v1: https://lore.kernel.org/git/20261113-http-backend-drop-strbuf-presizing-v1-1-7d66aae03b22@gmail.com/
---
Range-diff versus v1:

1:  9abfe3635b ! 1:  7561d02b83 http-backend: drop strbuf pre-sizing from get_info_packs()
    @@ Metadata
      ## Commit message ##
         http-backend: drop strbuf pre-sizing from get_info_packs()
     
    -    get_info_packs() iterates over all packs in the repository to count
    +    `get_info_packs()` iterates over all packs in the repository to count
         local packs, calls `strbuf_grow(&buf, cnt * 53 + 2)`, and then iterates
         over the packs a second time to format each entry with `strbuf_addf()`.
    -
         This pre-sizing dates back to 2f4038ab33 (Git-aware CGI to provide dumb
    -    HTTP transport, 2009-10-30) and is no longer worth keeping: `53` assumes
    -    the length of a SHA-1 "P pack-<hex>.pack\n" line (under-counting the
    -    77-byte lines in SHA-256 repositories), `cnt * 53 + 2` uses unchecked
    -    arithmetic, and `strbuf_addf()` / `strbuf_addch()` already grow the
    -    buffer automatically without needing an extra pass over the pack list.
    -    Similar pre-sizing was recently removed in a9ce8526dc (pretty: drop
    -    strbuf pre-sizing from add_rfc2047(), 2026-05-12).
    +    HTTP transport, 2009-10-30): `53` assumes the length of a SHA-1
    +    "P pack-<hex>.pack\n" line (under-counting the 77-byte lines in SHA-256
    +    repositories), and `cnt * 53 + 2` uses unchecked arithmetic.
    +
    +    Rather than fixing the size calculation with `the_hash_algo->hexsz` and
    +    `st_mult()` / `st_add()`, drop the pre-sizing altogether.
    +    `get_info_packs()` is a one-shot CGI handler for dumb HTTP
    +    `/objects/info/packs` requests that formats a single response and exits.
    +    A typical repository only has a few dozen packs (a few kilobytes of
    +    output), and even with thousands of packs `strbuf`'s geometric growth
    +    only performs a couple dozen reallocations. That cost is negligible next
    +    to CGI startup, scanning `objects/pack` to populate the pack list, and
    +    writing the HTTP response, whereas computing `cnt` up front requires an
    +    extra pass over the pack list.
     
    -    Drop `cnt`, the counting loop, and the `strbuf_grow()` call.
    +    Drop `cnt`, the counting loop, and the `strbuf_grow()` call, and let
    +    `strbuf_addf()` and `strbuf_addch()` grow the buffer as needed.
     
         Signed-off-by: Yusuke Hayashi <yusuke8h@gmail.com>
     
---
 http-backend.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/http-backend.c b/http-backend.c
index c7566b1d12..da9516a4c1 100644
--- a/http-backend.c
+++ b/http-backend.c
@@ -612,15 +612,8 @@ static void get_info_packs(struct strbuf *hdr, char *arg UNUSED)
 	size_t objdirlen = strlen(repo_get_object_directory(the_repository));
 	struct strbuf buf = STRBUF_INIT;
 	struct packed_git *p;
-	size_t cnt = 0;
 
 	select_getanyfile(hdr);
-	repo_for_each_pack(the_repository, p) {
-		if (p->pack_local)
-			cnt++;
-	}
-
-	strbuf_grow(&buf, cnt * 53 + 2);
 	repo_for_each_pack(the_repository, p) {
 		if (p->pack_local)
 			strbuf_addf(&buf, "P %s\n", p->pack_name + objdirlen + 6);

---
base-commit: 9354c5f04055932909b660a237905b360cba195a
change-id: 20261113-http-backend-drop-strbuf-presizing-6f496552d0ca