Git development
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: git@vger.kernel.org,  Luke Martin <lmartin@paramenoeng.com>
Subject: Re: [PATCH] pretty: drop strbuf pre-sizing from add_rfc2047()
Date: Wed, 13 May 2026 10:03:31 +0900	[thread overview]
Message-ID: <xmqqtsscjf30.fsf@gitster.g> (raw)
In-Reply-To: <20260512162022.GA69669@coredump.intra.peff.net> (Jeff King's message of "Tue, 12 May 2026 12:20:22 -0400")

Jeff King <peff@peff.net> writes:

> At the top of add_rfc2047() we do this:
>
>   strbuf_grow(sb, len * 3 + strlen(encoding) + 100);
>
> where "len" is the size of the header (like an author name) we are about
> to encode into the buffer. This pre-sizing is purely an optimization; we
> use strbuf_addf() and friends to actually write into the buffer, and
> they will grow the buffer as necessary.
> ...
> Is the optimization helping? I don't think so. Even for a gigantic case
> like the 1.4GB author name, I couldn't measure any slowdown when
> removing it. And most input will be much smaller, and added to a running
> strbuf containing the rest of the email-header output. We can just rely
> on strbuf's usual amortized-linear growth.
>
> So deleting the line seems like the best way to go. It eliminates the
> integer overflow and makes the code a tiny bit simpler.

Very nice.

Someday we may want to go through the output from

    $ git grep -e 'strbuf_grow(' \*.c

and remove this ineffective presizing.  I think any call to
strbuf_grow() that is immediately followed by a call to
strbuf_addX() is suspect, like in the following illustration (there
are others in these files).

diff --git c/apply.c w/apply.c
index 3de4aa4d2e..b0f146276e 100644
--- c/apply.c
+++ w/apply.c
@@ -3305,7 +3305,6 @@ static int apply_fragments(struct apply_state *state, struct image *img, struct
 static int read_blob_object(struct strbuf *buf, const struct object_id *oid, unsigned mode)
 {
 	if (S_ISGITLINK(mode)) {
-		strbuf_grow(buf, 100);
 		strbuf_addf(buf, "Subproject commit %s\n", oid_to_hex(oid));
 	} else {
 		enum object_type type;
diff --git c/archive.c w/archive.c
index fcd474c682..47b8725f0a 100644
--- c/archive.c
+++ w/archive.c
@@ -164,7 +164,6 @@ static int write_archive_entry(const struct object_id *oid, const char *base,
 
 	args->convert = 0;
 	strbuf_reset(&path);
-	strbuf_grow(&path, PATH_MAX);
 	strbuf_add(&path, args->base, args->baselen);
 	strbuf_add(&path, base, baselen);
 	strbuf_addstr(&path, filename);






  reply	other threads:[~2026-05-13  1:03 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-12 16:20 [PATCH] pretty: drop strbuf pre-sizing from add_rfc2047() Jeff King
2026-05-13  1:03 ` Junio C Hamano [this message]
2026-05-13 18:54   ` 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=xmqqtsscjf30.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=lmartin@paramenoeng.com \
    --cc=peff@peff.net \
    /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