git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [RFC PATCH 01/14] midx: use `string_list` for retained MIDX files

From
Junio C Hamano <gitster@pobox.com>
Date
Feb 26, 2026, 20:29 UTC
Message-ID
<xmqqldgf1c65.fsf@gitster.g>
In-Reply-To
<d64a799afd620363c1940d7c2e634e78ea553cb6.1771978829.git.me@ttaylorr.com>
Taylor Blau <me@ttaylorr.com> writes:
Show 9 quoted lines
> Both `clear_midx_files_ext()` and `clear_incremental_midx_files_ext()`
> build a list of filenames to keep while pruning stale MIDX files. Today
> they hand-roll an array instead of using a `string_list`, thus requiring
> us to pass an additional length parameter, and makes lookups linear.
>
> Replace the bare array with a `string_list` which can be passed around
> as a single parameter. Though it improves lookup performance, the
> difference is likely immeasurable given how small the keep_hashes array
> typically is.

And if it the lookup performance turns out to be an issue, we can switch to strmap or something more appropriate.

Show 106 quoted lines
>
> Signed-off-by: Taylor Blau <me@ttaylorr.com>
> ---
>  midx.c | 56 ++++++++++++++++++++++----------------------------------
>  1 file changed, 22 insertions(+), 34 deletions(-)
>
> diff --git a/midx.c b/midx.c
> index c1b9658240d..c5e3553e2bb 100644
> --- a/midx.c
> +++ b/midx.c
> @@ -755,8 +755,7 @@ int midx_checksum_valid(struct multi_pack_index *m)
>  }
>  
>  struct clear_midx_data {
> -	char **keep;
> -	uint32_t keep_nr;
> +	struct string_list keep;
>  	const char *ext;
>  };
>  
> @@ -764,15 +763,12 @@ static void clear_midx_file_ext(const char *full_path, size_t full_path_len UNUS
>  				const char *file_name, void *_data)
>  {
>  	struct clear_midx_data *data = _data;
> -	uint32_t i;
>  
>  	if (!(starts_with(file_name, "multi-pack-index-") &&
>  	      ends_with(file_name, data->ext)))
>  		return;
> -	for (i = 0; i < data->keep_nr; i++) {
> -		if (!strcmp(data->keep[i], file_name))
> -			return;
> -	}
> +	if (string_list_has_string(&data->keep, file_name))
> +		return;
>  	if (unlink(full_path))
>  		die_errno(_("failed to remove %s"), full_path);
>  }
> @@ -780,48 +776,40 @@ static void clear_midx_file_ext(const char *full_path, size_t full_path_len UNUS
>  void clear_midx_files_ext(struct odb_source *source, const char *ext,
>  			  const char *keep_hash)
>  {
> -	struct clear_midx_data data;
> -	memset(&data, 0, sizeof(struct clear_midx_data));
> -
> -	if (keep_hash) {
> -		ALLOC_ARRAY(data.keep, 1);
> -
> -		data.keep[0] = xstrfmt("multi-pack-index-%s.%s", keep_hash, ext);
> -		data.keep_nr = 1;
> -	}
> -	data.ext = ext;
> -
> -	for_each_file_in_pack_dir(source->path,
> -				  clear_midx_file_ext,
> -				  &data);
> +	struct clear_midx_data data = {
> +		.keep = STRING_LIST_INIT_NODUP,
> +		.ext = ext,
> +	};
>  
>  	if (keep_hash)
> -		free(data.keep[0]);
> -	free(data.keep);
> +		string_list_insert(&data.keep, xstrfmt("multi-pack-index-%s.%s",
> +						       keep_hash, ext));
> +
> +	for_each_file_in_pack_dir(source->path, clear_midx_file_ext, &data);
> +
> +	string_list_clear(&data.keep, 0);
>  }
>  
>  void clear_incremental_midx_files_ext(struct odb_source *source, const char *ext,
>  				      char **keep_hashes,
>  				      uint32_t hashes_nr)
>  {
> -	struct clear_midx_data data;
> +	struct clear_midx_data data = {
> +		.keep = STRING_LIST_INIT_NODUP,
> +		.ext = ext,
> +	};
>  	uint32_t i;
>  
> -	memset(&data, 0, sizeof(struct clear_midx_data));
> -
> -	ALLOC_ARRAY(data.keep, hashes_nr);
>  	for (i = 0; i < hashes_nr; i++)
> -		data.keep[i] = xstrfmt("multi-pack-index-%s.%s", keep_hashes[i],
> -				       ext);
> -	data.keep_nr = hashes_nr;
> -	data.ext = ext;
> +		string_list_append(&data.keep,
> +				   xstrfmt("multi-pack-index-%s.%s",
> +					   keep_hashes[i], ext));
> +	string_list_sort(&data.keep);
>  
>  	for_each_file_in_pack_subdir(source->path, "multi-pack-index.d",
>  				     clear_midx_file_ext, &data);
>  
> -	for (i = 0; i < hashes_nr; i++)
> -		free(data.keep[i]);
> -	free(data.keep);
> +	string_list_clear(&data.keep, 0);
>  }
>  
>  void clear_midx_file(struct repository *r)
Previous: Taylor BlauNext: Taylor Blau
Message 5 of 21 in “repack: incremental MIDX/bitmap-based repacking”
  1. 00/14 repack: incremental MIDX/bitmap-based repackingTaylor Blau, Feb 25, 2026
  2. 06/14 repack: track the ODB source via existing_packsTaylor Blau, Feb 25, 2026
  3. Taylor BlauFeb 25, 2026
  4. 01/14 midx: use `string_list` for retained MIDX filesTaylor Blau, Feb 25, 2026
  5. Junio C HamanoFeb 26, 2026
  6. Taylor BlauFeb 27, 2026
  7. 02/14 strvec: introduce `strvec_init_alloc()`Taylor Blau, Feb 25, 2026
  8. Junio C HamanoFeb 26, 2026
  9. Junio C HamanoFeb 26, 2026
  10. Taylor BlauFeb 27, 2026
  11. 03/14 midx: use `strvec` for `keep_hashes`Taylor Blau, Feb 25, 2026
  12. 04/14 midx: introduce `--checksum-only` for incremental MIDX writesTaylor Blau, Feb 25, 2026
  13. 05/14 midx: support custom `--base` for incremental MIDX writesTaylor Blau, Feb 25, 2026
  14. 07/14 midx: expose `midx_layer_contains_pack()`Taylor Blau, Feb 25, 2026
  15. 08/14 repack-midx: factor out `repack_prepare_midx_command()`Taylor Blau, Feb 25, 2026
  16. 09/14 repack-midx: extract `repack_fill_midx_stdin_packs()`Taylor Blau, Feb 25, 2026
  17. 10/14 repack-geometry: prepare for incremental MIDX repackingTaylor Blau, Feb 25, 2026
  18. 11/14 builtin/repack.c: convert `--write-midx` to an `OPT_CALLBACK`Taylor Blau, Feb 25, 2026
  19. 12/14 repack: implement incremental MIDX repackingTaylor Blau, Feb 25, 2026
  20. 13/14 repack: introduce `--write-midx=incremental`Taylor Blau, Feb 25, 2026
  21. 14/14 repack: allow `--write-midx=incremental` without `--geometric`Taylor Blau, Feb 25, 2026

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.