From: Jeff King <peff@peff.net>
To: Taylor Blau <me@ttaylorr.com>
Cc: git@vger.kernel.org, Junio C Hamano <gitster@pobox.com>,
Elijah Newren <newren@gmail.com>, Patrick Steinhardt <ps@pks.im>
Subject: Re: [PATCH 03/16] strvec: introduce `strvec_init_alloc()`
Date: Mon, 30 Mar 2026 18:46:31 -0400 [thread overview]
Message-ID: <20260330224631.GC41843@coredump.intra.peff.net> (raw)
In-Reply-To: <aea6aad695336a974df9c90ce7bd02ddcc33582f.1774820449.git.me@ttaylorr.com>
On Sun, Mar 29, 2026 at 05:40:58PM -0400, Taylor Blau wrote:
> When the caller knows upfront how many elements will be pushed onto a
> `strvec`, it is useful to pre-allocate enough space in the array to fit
> that many elements (and one additional slot to store NULL, indicating
> the end of the list.)
The main reason I didn't add an "alloc" parameter to the initializer
originally is that if you know how many elements you are going to push,
you probably don't need a strvec in the first place (you can just make
an array). But let's wait and see how it gets used in a future patch.
> +void strvec_init_alloc(struct strvec *array, size_t alloc)
> +{
> + if (!alloc) {
> + struct strvec blank = STRVEC_INIT;
> + memcpy(array, &blank, sizeof(*array));
> + } else {
> + CALLOC_ARRAY(array->v, st_add(alloc, 1));
> + array->nr = 0;
> + array->alloc = alloc + 1;
> + }
> }
We use the blank+memcpy approach so that we don't duplicate knowledge of
which elements are in the struct (and which might need to be
initialized). But now there's an implicit zero-ing happening when alloc
is non-zero. Can we write this as:
struct strvec blank = STRVEC_INIT;
memcpy(array, &blank, sizeof(*array));
if (alloc) {
CALLOC_ARRAY(array->v, st_add(alloc, 1));
array->alloc = alloc + 1;
}
The worst case is a useless write of a few struct elements, but I
suspect that an optimizing compiler can even get rid of those.
I doubt we are going to add new elements to a strvec, so this is
arguably just pedantry. It just feels like it's easy enough to do now
while we're thinking about it.
As an aside, it is sad that we have to CALLOC() ourselves here, as
there is no reusable "grow" operation. Probably not worth refactoring
it out, though, though.
-Peff
next prev parent reply other threads:[~2026-03-30 22:46 UTC|newest]
Thread overview: 92+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-03-29 21:40 [PATCH 00/16] repack: incremental MIDX/bitmap-based repacking Taylor Blau
2026-03-29 21:40 ` [PATCH 01/16] midx-write: handle noop writes when converting incremental chains Taylor Blau
2026-03-30 22:33 ` Jeff King
2026-03-31 21:43 ` Taylor Blau
2026-03-29 21:40 ` [PATCH 02/16] midx: use `string_list` for retained MIDX files Taylor Blau
2026-03-30 22:38 ` Jeff King
2026-03-31 21:49 ` Taylor Blau
2026-03-29 21:40 ` [PATCH 03/16] strvec: introduce `strvec_init_alloc()` Taylor Blau
2026-03-30 22:46 ` Jeff King [this message]
2026-03-29 21:41 ` [PATCH 04/16] midx: use `strvec` for `keep_hashes` Taylor Blau
2026-03-30 23:01 ` Jeff King
2026-03-31 22:26 ` Taylor Blau
2026-03-31 22:50 ` Taylor Blau
2026-03-31 23:17 ` Jeff King
2026-04-01 15:41 ` Taylor Blau
2026-04-01 19:25 ` Jeff King
2026-03-29 21:41 ` [PATCH 05/16] midx: introduce `--checksum-only` for incremental MIDX writes Taylor Blau
2026-03-30 23:15 ` Jeff King
2026-04-02 22:51 ` Taylor Blau
2026-03-29 21:41 ` [PATCH 06/16] midx: support custom `--base` " Taylor Blau
2026-04-07 5:57 ` Jeff King
2026-04-14 22:09 ` Taylor Blau
2026-03-29 21:41 ` [PATCH 07/16] repack: track the ODB source via existing_packs Taylor Blau
2026-04-07 6:04 ` Jeff King
2026-04-14 22:24 ` Taylor Blau
2026-03-29 21:41 ` [PATCH 08/16] midx: expose `midx_layer_contains_pack()` Taylor Blau
2026-04-07 6:05 ` Jeff King
2026-03-29 21:41 ` [PATCH 09/16] repack-midx: factor out `repack_prepare_midx_command()` Taylor Blau
2026-03-29 21:41 ` [PATCH 10/16] repack-midx: extract `repack_fill_midx_stdin_packs()` Taylor Blau
2026-04-07 6:08 ` Jeff King
2026-03-29 21:41 ` [PATCH 11/16] repack-geometry: prepare for incremental MIDX repacking Taylor Blau
2026-04-07 6:10 ` Jeff King
2026-04-16 22:51 ` Elijah Newren
2026-04-21 19:34 ` Taylor Blau
2026-03-29 21:41 ` [PATCH 12/16] builtin/repack.c: convert `--write-midx` to an `OPT_CALLBACK` Taylor Blau
2026-04-07 6:18 ` Jeff King
2026-03-29 21:41 ` [PATCH 13/16] packfile: ensure `close_pack_revindex()` frees in-memory revindex Taylor Blau
2026-04-07 6:29 ` Jeff King
2026-03-29 21:41 ` [PATCH 14/16] repack: implement incremental MIDX repacking Taylor Blau
2026-04-16 22:53 ` Elijah Newren
2026-04-21 19:40 ` Taylor Blau
2026-03-29 21:41 ` [PATCH 15/16] repack: introduce `--write-midx=incremental` Taylor Blau
2026-04-16 22:53 ` Elijah Newren
2026-04-21 19:52 ` Taylor Blau
2026-03-29 21:41 ` [PATCH 16/16] repack: allow `--write-midx=incremental` without `--geometric` Taylor Blau
2026-04-14 22:38 ` [PATCH 00/16] repack: incremental MIDX/bitmap-based repacking Taylor Blau
2026-04-21 20:37 ` [PATCH v2 " Taylor Blau
2026-04-21 20:37 ` [PATCH v2 01/16] midx-write: handle noop writes when converting incremental chains Taylor Blau
2026-04-21 20:37 ` [PATCH v2 02/16] midx: use `strset` for retained MIDX files Taylor Blau
2026-04-21 20:37 ` [PATCH v2 03/16] midx: build `keep_hashes` array in order Taylor Blau
2026-04-21 20:37 ` [PATCH v2 04/16] midx: use `strvec` for `keep_hashes` Taylor Blau
2026-04-21 20:37 ` [PATCH v2 05/16] midx: introduce `--no-write-chain-file` for incremental MIDX writes Taylor Blau
2026-04-21 20:37 ` [PATCH v2 06/16] midx: support custom `--base` " Taylor Blau
2026-04-21 20:37 ` [PATCH v2 07/16] repack: track the ODB source via existing_packs Taylor Blau
2026-04-21 20:37 ` [PATCH v2 08/16] midx: expose `midx_layer_contains_pack()` Taylor Blau
2026-04-21 20:37 ` [PATCH v2 09/16] repack-midx: factor out `repack_prepare_midx_command()` Taylor Blau
2026-04-21 20:37 ` [PATCH v2 10/16] repack-midx: extract `repack_fill_midx_stdin_packs()` Taylor Blau
2026-04-29 8:08 ` Jeff King
2026-04-29 22:40 ` Taylor Blau
2026-04-21 20:37 ` [PATCH v2 11/16] repack-geometry: prepare for incremental MIDX repacking Taylor Blau
2026-04-21 20:37 ` [PATCH v2 12/16] builtin/repack.c: convert `--write-midx` to an `OPT_CALLBACK` Taylor Blau
2026-04-21 20:37 ` [PATCH v2 13/16] packfile: ensure `close_pack_revindex()` frees in-memory revindex Taylor Blau
2026-04-21 20:37 ` [PATCH v2 14/16] repack: implement incremental MIDX repacking Taylor Blau
2026-04-29 7:51 ` Jeff King
2026-04-29 23:36 ` Taylor Blau
2026-04-29 8:10 ` Jeff King
2026-04-29 23:39 ` Taylor Blau
2026-04-21 20:37 ` [PATCH v2 15/16] repack: introduce `--write-midx=incremental` Taylor Blau
2026-04-21 21:02 ` Taylor Blau
2026-04-21 20:38 ` [PATCH v2 16/16] repack: allow `--write-midx=incremental` without `--geometric` Taylor Blau
2026-04-22 14:45 ` [PATCH v2 00/16] repack: incremental MIDX/bitmap-based repacking Elijah Newren
2026-04-29 8:10 ` Jeff King
2026-04-30 0:13 ` [PATCH v3 " Taylor Blau
2026-04-30 0:13 ` [PATCH v3 01/16] midx-write: handle noop writes when converting incremental chains Taylor Blau
2026-04-30 0:13 ` [PATCH v3 02/16] midx: use `strset` for retained MIDX files Taylor Blau
2026-04-30 0:13 ` [PATCH v3 03/16] midx: build `keep_hashes` array in order Taylor Blau
2026-04-30 0:13 ` [PATCH v3 04/16] midx: use `strvec` for `keep_hashes` Taylor Blau
2026-04-30 0:13 ` [PATCH v3 05/16] midx: introduce `--no-write-chain-file` for incremental MIDX writes Taylor Blau
2026-04-30 0:13 ` [PATCH v3 06/16] midx: support custom `--base` " Taylor Blau
2026-04-30 0:13 ` [PATCH v3 07/16] repack: track the ODB source via existing_packs Taylor Blau
2026-04-30 0:13 ` [PATCH v3 08/16] midx: expose `midx_layer_contains_pack()` Taylor Blau
2026-04-30 0:13 ` [PATCH v3 09/16] repack-midx: factor out `repack_prepare_midx_command()` Taylor Blau
2026-05-13 21:45 ` SZEDER Gábor
2026-04-30 0:13 ` [PATCH v3 10/16] repack-midx: extract `repack_fill_midx_stdin_packs()` Taylor Blau
2026-04-30 0:13 ` [PATCH v3 11/16] repack-geometry: prepare for incremental MIDX repacking Taylor Blau
2026-04-30 0:13 ` [PATCH v3 12/16] builtin/repack.c: convert `--write-midx` to an `OPT_CALLBACK` Taylor Blau
2026-04-30 0:13 ` [PATCH v3 13/16] packfile: ensure `close_pack_revindex()` frees in-memory revindex Taylor Blau
2026-04-30 0:13 ` [PATCH v3 14/16] repack: implement incremental MIDX repacking Taylor Blau
2026-04-30 0:13 ` [PATCH v3 15/16] repack: introduce `--write-midx=incremental` Taylor Blau
2026-05-13 23:08 ` Jeff King
2026-04-30 0:13 ` [PATCH v3 16/16] repack: allow `--write-midx=incremental` without `--geometric` Taylor Blau
2026-05-01 6:46 ` [PATCH v3 00/16] repack: incremental MIDX/bitmap-based repacking 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=20260330224631.GC41843@coredump.intra.peff.net \
--to=peff@peff.net \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=me@ttaylorr.com \
--cc=newren@gmail.com \
--cc=ps@pks.im \
/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