{"thread":{"id":"65809","subject":"[PATCH] cat-file: speed up default format","startedAt":"2026-06-14T16:28:36Z","lastAt":"2026-06-16T22:21:27Z","messageCount":10,"participants":["René Scharfe","Patrick Steinhardt","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"545490","messageId":"5a7ed929-6fe0-496c-83bd-65dee57c2241@web.de","threadId":"65809","inReplyTo":null,"subject":"[PATCH] cat-file: speed up default format","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2026-06-14T16:28:34Z","receivedAt":"2026-06-14T16:28:36Z","isPatch":true,"body":"eb54a3391b (cat-file: skip expanding default format, 2022-03-15) added\nspecial handling for the default batch format.  In the meantime it has\nfallen behind the code path for handling arbitrary formats.  Bring it up\nto speed by using the new and more efficient strbuf_add_oid_hex() and\nstrbuf_add_uint() instead of strbuf_addf():\n\nBenchmark 1: ./git_main cat-file --batch-all-objects --batch-check='%(objectname) %(objecttype) %(objectsize)'\n  Time (mean ± σ):      1.051 s ±  0.003 s    [User: 1.027 s, System: 0.023 s]\n  Range (min … max):    1.049 s …  1.058 s    10 runs\n\nBenchmark 2: ./git_main cat-file --batch-all-objects --batch-check='%(objectname)-%(objecttype)-%(objectsize)'\n  Time (mean ± σ):      1.012 s ±  0.002 s    [User: 0.988 s, System: 0.023 s]\n  Range (min … max):    1.010 s …  1.018 s    10 runs\n\nBenchmark 3: ./git cat-file --batch-all-objects --batch-check='%(objectname) %(objecttype) %(objectsize)'\n  Time (mean ± σ):     979.0 ms ±   1.1 ms    [User: 954.1 ms, System: 23.2 ms]\n  Range (min … max):   977.7 ms … 980.8 ms    10 runs\n\nSummary\n  ./git cat-file --batch-all-objects --batch-check='%(objectname) %(objecttype) %(objectsize)' ran\n    1.03 ± 0.00 times faster than ./git_main cat-file --batch-all-objects --batch-check='%(objectname)-%(objecttype)-%(objectsize)'\n    1.07 ± 0.00 times faster than ./git_main cat-file --batch-all-objects --batch-check='%(objectname) %(objecttype) %(objectsize)'\n\nSigned-off-by: René Scharfe <l.s.r@web.de>\n---\n builtin/cat-file.c | 9 ++++++---\n 1 file changed, 6 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/cat-file.c b/builtin/cat-file.c\nindex 2b64f8f733..d7f7895e30 100644\n--- a/builtin/cat-file.c\n+++ b/builtin/cat-file.c\n@@ -461,9 +461,12 @@ static void print_object_or_die(struct batch_options *opt, struct expand_data *d\n static void print_default_format(struct strbuf *scratch, struct expand_data *data,\n \t\t\t\t struct batch_options *opt)\n {\n-\tstrbuf_addf(scratch, \"%s %s %\"PRIuMAX\"%c\", oid_to_hex(&data->oid),\n-\t\t    type_name(data->type),\n-\t\t    (uintmax_t)data->size, opt->output_delim);\n+\tstrbuf_add_oid_hex(scratch, &data->oid);\n+\tstrbuf_addch(scratch, ' ');\n+\tstrbuf_addstr(scratch, type_name(data->type));\n+\tstrbuf_addch(scratch, ' ');\n+\tstrbuf_add_uint(scratch, data->size);\n+\tstrbuf_addch(scratch, opt->output_delim);\n }\n \n static void report_object_status(struct batch_options *opt,\n-- \n2.54.0\n"},{"id":"545518","messageId":"ai-paIFWuVzQ_yx_@pks.im","threadId":"65809","inReplyTo":"5a7ed929-6fe0-496c-83bd-65dee57c2241@web.de","subject":"Re: [PATCH] cat-file: speed up default format","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-06-15T07:27:36Z","receivedAt":"2026-06-15T07:27:42Z","isPatch":true,"body":"On Sun, Jun 14, 2026 at 06:28:34PM +0200, René Scharfe wrote:\n> eb54a3391b (cat-file: skip expanding default format, 2022-03-15) added\n> special handling for the default batch format.  In the meantime it has\n> fallen behind the code path for handling arbitrary formats.  Bring it up\n> to speed by using the new and more efficient strbuf_add_oid_hex() and\n> strbuf_add_uint() instead of strbuf_addf():\n> \n> Benchmark 1: ./git_main cat-file --batch-all-objects --batch-check='%(objectname) %(objecttype) %(objectsize)'\n>   Time (mean ± σ):      1.051 s ±  0.003 s    [User: 1.027 s, System: 0.023 s]\n>   Range (min … max):    1.049 s …  1.058 s    10 runs\n> \n> Benchmark 2: ./git_main cat-file --batch-all-objects --batch-check='%(objectname)-%(objecttype)-%(objectsize)'\n>   Time (mean ± σ):      1.012 s ±  0.002 s    [User: 0.988 s, System: 0.023 s]\n>   Range (min … max):    1.010 s …  1.018 s    10 runs\n> \n> Benchmark 3: ./git cat-file --batch-all-objects --batch-check='%(objectname) %(objecttype) %(objectsize)'\n>   Time (mean ± σ):     979.0 ms ±   1.1 ms    [User: 954.1 ms, System: 23.2 ms]\n>   Range (min … max):   977.7 ms … 980.8 ms    10 runs\n> \n> Summary\n>   ./git cat-file --batch-all-objects --batch-check='%(objectname) %(objecttype) %(objectsize)' ran\n>     1.03 ± 0.00 times faster than ./git_main cat-file --batch-all-objects --batch-check='%(objectname)-%(objecttype)-%(objectsize)'\n>     1.07 ± 0.00 times faster than ./git_main cat-file --batch-all-objects --batch-check='%(objectname) %(objecttype) %(objectsize)'\n\nThis almost makes me wonder whether it even makes sense to keep around\nthe handler for the default format. Is a 3% speedup worth the additional\ncomplexity and the need to keep those sites in sync?\n\n> diff --git a/builtin/cat-file.c b/builtin/cat-file.c\n> index 2b64f8f733..d7f7895e30 100644\n> --- a/builtin/cat-file.c\n> +++ b/builtin/cat-file.c\n> @@ -461,9 +461,12 @@ static void print_object_or_die(struct batch_options *opt, struct expand_data *d\n>  static void print_default_format(struct strbuf *scratch, struct expand_data *data,\n>  \t\t\t\t struct batch_options *opt)\n>  {\n> -\tstrbuf_addf(scratch, \"%s %s %\"PRIuMAX\"%c\", oid_to_hex(&data->oid),\n> -\t\t    type_name(data->type),\n> -\t\t    (uintmax_t)data->size, opt->output_delim);\n> +\tstrbuf_add_oid_hex(scratch, &data->oid);\n> +\tstrbuf_addch(scratch, ' ');\n> +\tstrbuf_addstr(scratch, type_name(data->type));\n> +\tstrbuf_addch(scratch, ' ');\n> +\tstrbuf_add_uint(scratch, data->size);\n> +\tstrbuf_addch(scratch, opt->output_delim);\n>  }\n\nThe change itself looks obviously good to me though, thanks!\n\nPatrick\n"},{"id":"545600","messageId":"20260615165326.GA91269@coredump.intra.peff.net","threadId":"65809","inReplyTo":"5a7ed929-6fe0-496c-83bd-65dee57c2241@web.de","subject":"Re: [PATCH] cat-file: speed up default format","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-06-15T16:53:26Z","receivedAt":"2026-06-15T16:53:35Z","isPatch":true,"body":"On Sun, Jun 14, 2026 at 06:28:34PM +0200, René Scharfe wrote:\n\n> eb54a3391b (cat-file: skip expanding default format, 2022-03-15) added\n> special handling for the default batch format.  In the meantime it has\n> fallen behind the code path for handling arbitrary formats.  Bring it up\n> to speed by using the new and more efficient strbuf_add_oid_hex() and\n> strbuf_add_uint() instead of strbuf_addf():\n> \n> Benchmark 1: ./git_main cat-file --batch-all-objects --batch-check='%(objectname) %(objecttype) %(objectsize)'\n>   Time (mean ± σ):      1.051 s ±  0.003 s    [User: 1.027 s, System: 0.023 s]\n>   Range (min … max):    1.049 s …  1.058 s    10 runs\n> \n> Benchmark 2: ./git_main cat-file --batch-all-objects --batch-check='%(objectname)-%(objecttype)-%(objectsize)'\n>   Time (mean ± σ):      1.012 s ±  0.002 s    [User: 0.988 s, System: 0.023 s]\n>   Range (min … max):    1.010 s …  1.018 s    10 runs\n> \n> Benchmark 3: ./git cat-file --batch-all-objects --batch-check='%(objectname) %(objecttype) %(objectsize)'\n>   Time (mean ± σ):     979.0 ms ±   1.1 ms    [User: 954.1 ms, System: 23.2 ms]\n>   Range (min … max):   977.7 ms … 980.8 ms    10 runs\n\nInteresting that it was actually slower than a custom format.  Using the\ndefault format saves the cost of strbuf_expand(), but it was paying the\nprice of strbuf_addf(), which the custom path no longer used. So the\ncost of strbuf_addf() is more than strbuf_expand(), which is not all\nthat surprising.\n\nYour patch seems obviously right, and everything below is idle\nspeculation / nerd-sniping.\n\nI have long wondered if we could do better with a separate initial parse\nstep, which would let us walk the parse tree for each object. In theory\nthat tree is more compact.\n\nI think it would be a huge improvement for ref-filter, whose parser is\ncomplicated and slow (though its biggest sin is that it allocates a\nseparate string for each atom before assembling the final output). But\ncould it help even cat-file, which is using a pretty tight loop over\nstrbuf_expand()? I sketched out a rough draft below.\n\nIt uses per-atom callback functions which is nice and clean, though we\nmight be able to do even better with a big ugly switch() statement.\n\nThe timings I got are below (git.old is master with your patch here\napplied, and git.new is my patch on top). It looks like it does make a\ncustom format ~3% faster. But it's still a shade slower than the default\nformat. Not sure if it's the extra function calls, or if the static\nprint_default_format() function gives the compiler more opportunities\nfor optimization.\n\n  Benchmark 1: ./git.old cat-file --batch-all-objects --batch-check='%(objectname) %(objecttype) %(objectsize)'\n    Time (mean ± σ):     580.2 ms ±   5.0 ms    [User: 558.7 ms, System: 21.5 ms]\n    Range (min … max):   569.9 ms … 585.6 ms    10 runs\n  \n  Benchmark 2: ./git.new cat-file --batch-all-objects --batch-check='%(objectname) %(objecttype) %(objectsize)'\n    Time (mean ± σ):     580.4 ms ±   5.1 ms    [User: 562.7 ms, System: 17.8 ms]\n    Range (min … max):   571.8 ms … 587.0 ms    10 runs\n  \n  Benchmark 3: ./git.old cat-file --batch-all-objects --batch-check='%(objectname)-%(objecttype)-%(objectsize)'\n    Time (mean ± σ):     618.6 ms ±   5.0 ms    [User: 598.9 ms, System: 19.7 ms]\n    Range (min … max):   613.6 ms … 628.3 ms    10 runs\n  \n  Benchmark 4: ./git.new cat-file --batch-all-objects --batch-check='%(objectname)-%(objecttype)-%(objectsize)'\n    Time (mean ± σ):     600.2 ms ±   4.2 ms    [User: 581.2 ms, System: 19.0 ms]\n    Range (min … max):   595.2 ms … 608.8 ms    10 runs\n  \n  Summary\n    ./git.old cat-file --batch-all-objects --batch-check='%(objectname) %(objecttype) %(objectsize)' ran\n      1.00 ± 0.01 times faster than ./git.new cat-file --batch-all-objects --batch-check='%(objectname) %(objecttype) %(objectsize)'\n      1.03 ± 0.01 times faster than ./git.new cat-file --batch-all-objects --batch-check='%(objectname)-%(objecttype)-%(objectsize)'\n      1.07 ± 0.01 times faster than ./git.old cat-file --batch-all-objects --batch-check='%(objectname)-%(objecttype)-%(objectsize)'\n\nPatch below, only lightly tested.\n\n---\ndiff --git a/builtin/cat-file.c b/builtin/cat-file.c\nindex d7f7895e30..9cc7ec7a6f 100644\n--- a/builtin/cat-file.c\n+++ b/builtin/cat-file.c\n@@ -36,6 +36,8 @@ enum batch_mode {\n \tBATCH_MODE_QUEUE_AND_DISPATCH,\n };\n \n+struct format_item;\n+\n struct batch_options {\n \tstruct list_objects_filter_options objects_filter;\n \tint enabled;\n@@ -48,6 +50,7 @@ struct batch_options {\n \tchar input_delim;\n \tchar output_delim;\n \tconst char *format;\n+\tstruct format_item *parsed_format;\n };\n \n static const char *force_path;\n@@ -294,12 +297,6 @@ struct expand_data {\n \tconst char *rest;\n \tstruct object_id delta_base_oid;\n \n-\t/*\n-\t * If mark_query is true, we do not expand anything, but rather\n-\t * just mark the object_info with items we wish to query.\n-\t */\n-\tint mark_query;\n-\n \t/*\n \t * Whether to split the input on whitespace before feeding it to\n \t * get_sha1; this is decided during the mark_query phase based on\n@@ -323,65 +320,152 @@ struct expand_data {\n };\n #define EXPAND_DATA_INIT  { .mode = S_IFINVALID }\n \n+struct format_item {\n+\tvoid (*add)(struct format_item *item, struct strbuf *sb, struct expand_data *data);\n+\tunion {\n+\t\tstruct {\n+\t\t\tconst char *p;\n+\t\t\tsize_t len;\n+\t\t} literal;\n+\t} u;\n+\t/*\n+\t * We could make a true tree here with child/next pointers, which would\n+\t * be necessary if we had recursive formats, like %(if). But for our\n+\t * simple formats for now it is enough to have a linear set of items,\n+\t * so we'll just allocate an array and terminate it with a NULL entry.\n+\t */\n+};\n+\n+static void objectname_add(struct format_item *item UNUSED,\n+\t\t\t   struct strbuf *sb, struct expand_data *data)\n+{\n+\tstrbuf_add_oid_hex(sb, &data->oid);\n+}\n+\n+static void objecttype_add(struct format_item *item UNUSED,\n+\t\t\t   struct strbuf *sb, struct expand_data *data)\n+{\n+\tstrbuf_addstr(sb, type_name(data->type));\n+}\n+\n+static void objectsize_add(struct format_item *item UNUSED,\n+\t\t\t   struct strbuf *sb, struct expand_data *data)\n+{\n+\tstrbuf_add_uint(sb, data->size);\n+}\n+\n+static void objectsize_disk_add(struct format_item *item UNUSED,\n+\t\t\t\tstruct strbuf *sb, struct expand_data *data)\n+{\n+\tstrbuf_add_uint(sb, data->disk_size);\n+}\n+\n+static void rest_add(struct format_item *item UNUSED,\n+\t\t     struct strbuf *sb, struct expand_data *data)\n+{\n+\tstrbuf_addstr(sb, data->rest);\n+}\n+\n+static void deltabase_add(struct format_item *item UNUSED,\n+\t\t\t  struct strbuf *sb, struct expand_data *data)\n+{\n+\tstrbuf_add_oid_hex(sb, &data->delta_base_oid);\n+}\n+\n+static void objectmode_add(struct format_item *item UNUSED,\n+\t\t\t   struct strbuf *sb, struct expand_data *data)\n+{\n+\tif (data->mode != S_IFINVALID)\n+\t\tstrbuf_addf(sb, \"%06o\", data->mode);\n+}\n+\n+static void literal_add(struct format_item *item,\n+\t\t\tstruct strbuf *sb, struct expand_data *data UNUSED)\n+{\n+\tstrbuf_add(sb, item->u.literal.p, item->u.literal.len);\n+}\n+\n static int is_atom(const char *atom, const char *s, int slen)\n {\n \tint alen = strlen(atom);\n \treturn alen == slen && !memcmp(atom, s, alen);\n }\n \n-static int expand_atom(struct strbuf *sb, const char *atom, int len,\n-\t\t       struct expand_data *data)\n+static int parse_atom(struct format_item *fmt, const char *atom, int len,\n+\t\t      struct expand_data *data)\n {\n \tif (is_atom(\"objectname\", atom, len)) {\n-\t\tif (!data->mark_query)\n-\t\t\tstrbuf_add_oid_hex(sb, &data->oid);\n+\t\tfmt->add = objectname_add;\n \t} else if (is_atom(\"objecttype\", atom, len)) {\n-\t\tif (data->mark_query)\n-\t\t\tdata->info.typep = &data->type;\n-\t\telse\n-\t\t\tstrbuf_addstr(sb, type_name(data->type));\n+\t\tdata->info.typep = &data->type;\n+\t\tfmt->add = objecttype_add;\n \t} else if (is_atom(\"objectsize\", atom, len)) {\n-\t\tif (data->mark_query)\n-\t\t\tdata->info.sizep = &data->size;\n-\t\telse\n-\t\t\tstrbuf_add_uint(sb, data->size);\n+\t\tdata->info.sizep = &data->size;\n+\t\tfmt->add = objectsize_add;\n \t} else if (is_atom(\"objectsize:disk\", atom, len)) {\n-\t\tif (data->mark_query)\n-\t\t\tdata->info.disk_sizep = &data->disk_size;\n-\t\telse\n-\t\t\tstrbuf_add_uint(sb, data->disk_size);\n+\t\tdata->info.disk_sizep = &data->disk_size;\n+\t\tfmt->add = objectsize_disk_add;\n \t} else if (is_atom(\"rest\", atom, len)) {\n-\t\tif (data->mark_query)\n-\t\t\tdata->split_on_whitespace = 1;\n-\t\telse if (data->rest)\n-\t\t\tstrbuf_addstr(sb, data->rest);\n+\t\tdata->split_on_whitespace = 1;\n+\t\tfmt->add = rest_add;\n \t} else if (is_atom(\"deltabase\", atom, len)) {\n-\t\tif (data->mark_query)\n-\t\t\tdata->info.delta_base_oid = &data->delta_base_oid;\n-\t\telse\n-\t\t\tstrbuf_add_oid_hex(sb, &data->delta_base_oid);\n+\t\tdata->info.delta_base_oid = &data->delta_base_oid;\n+\t\tfmt->add = deltabase_add;\n \t} else if (is_atom(\"objectmode\", atom, len)) {\n-\t\tif (!data->mark_query && !(S_IFINVALID == data->mode))\n-\t\t\tstrbuf_addf(sb, \"%06o\", data->mode);\n+\t\tfmt->add = objectmode_add;\n \t} else\n \t\treturn 0;\n \treturn 1;\n }\n \n-static void expand_format(struct strbuf *sb, const char *start,\n-\t\t\t  struct expand_data *data)\n+static struct format_item *parse_format(const char *start,\n+\t\t\t\t\tstruct expand_data *data)\n {\n-\twhile (strbuf_expand_step(sb, &start)) {\n+\tstruct format_item *ret = NULL;\n+\tsize_t nr = 0, alloc = 0;\n+\n+\twhile (1) {\n+\t\tconst char *percent = strchrnul(start, '%');\n \t\tconst char *end;\n \n-\t\tif (skip_prefix(start, \"%\", &start) || *start != '(')\n-\t\t\tstrbuf_addch(sb, '%');\n-\t\telse if ((end = strchr(start + 1, ')')) &&\n-\t\t\t expand_atom(sb, start + 1, end - start - 1, data))\n+\t\tif (percent != start) {\n+\t\t\tALLOC_GROW(ret, nr + 1, alloc);\n+\t\t\tret[nr].add = literal_add;\n+\t\t\tret[nr].u.literal.p = start;\n+\t\t\tret[nr].u.literal.len = percent - start;\n+\t\t\tnr++;\n+\t\t}\n+\n+\t\tif (!*percent)\n+\t\t\tbreak;\n+\n+\t\tstart = percent + 1;\n+\n+\t\tALLOC_GROW(ret, nr + 1, alloc);\n+\t\tif (skip_prefix(start, \"%\", &start) || *start != '(') {\n+\t\t\tret[nr].add = literal_add;\n+\t\t\tret[nr].u.literal.p = \"%\";\n+\t\t\tret[nr].u.literal.len = 1;\n+\t\t} else if ((end = strchr(start + 1, ')')) &&\n+\t\t\t   parse_atom(&ret[nr], start + 1, end - start - 1, data)) {\n \t\t\tstart = end + 1;\n-\t\telse\n+\t\t} else {\n \t\t\tstrbuf_expand_bad_format(start, \"cat-file\");\n+\t\t}\n+\t\tnr++;\n \t}\n+\n+\tALLOC_GROW(ret, nr + 1, alloc);\n+\tret[nr].add = NULL;\n+\n+\treturn ret;\n+}\n+\n+static void expand_format(struct strbuf *sb, struct format_item *fmt,\n+\t\t\t  struct expand_data *data)\n+{\n+\tfor (; fmt->add; fmt++)\n+\t\tfmt->add(fmt, sb, data);\n }\n \n static void batch_write(struct batch_options *opt, const void *data, int len)\n@@ -568,7 +652,7 @@ static void batch_object_write(const char *obj_name,\n \tif (!opt->format) {\n \t\tprint_default_format(scratch, data, opt);\n \t} else {\n-\t\texpand_format(scratch, opt->format, data);\n+\t\texpand_format(scratch, opt->parsed_format, data);\n \t\tstrbuf_addch(scratch, opt->output_delim);\n \t}\n \n@@ -936,17 +1020,9 @@ static int batch_objects(struct batch_options *opt)\n \tint save_warning;\n \tint retval = 0;\n \n-\t/*\n-\t * Expand once with our special mark_query flag, which will prime the\n-\t * object_info to be handed to odb_read_object_info_extended for each\n-\t * object.\n-\t */\n-\tdata.mark_query = 1;\n-\texpand_format(&output,\n-\t\t      opt->format ? opt->format : DEFAULT_FORMAT,\n-\t\t      &data);\n-\tdata.mark_query = 0;\n-\tstrbuf_release(&output);\n+\topt->parsed_format = parse_format(opt->format ?\n+\t\t\t\t\t  opt->format : DEFAULT_FORMAT,\n+\t\t\t\t\t  &data);\n \tif (opt->transform_mode)\n \t\tdata.split_on_whitespace = 1;\n \n"},{"id":"545602","messageId":"20260615170652.GB91269@coredump.intra.peff.net","threadId":"65809","inReplyTo":"20260615165326.GA91269@coredump.intra.peff.net","subject":"Re: [PATCH] cat-file: speed up default format","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-06-15T17:06:52Z","receivedAt":"2026-06-15T17:06:54Z","isPatch":true,"body":"On Mon, Jun 15, 2026 at 12:53:26PM -0400, Jeff King wrote:\n\n> It uses per-atom callback functions which is nice and clean, though we\n> might be able to do even better with a big ugly switch() statement.\n\nBeing the curious sort, I swapped it out for a big switch statement.\nPatch below, but it does not seem to be any faster.\n\nSo the bottom line is I think you could gain a little bit of performance\nby pre-parsing (versus strbuf_expand() on each object). Around 3% for\nsomething that actually looks at the objects, though more like 15% if\nfor just dumping the objectnames.\n\nIMHO that is probably not worth it for a custom parsing system just for\ncat-file.  But if we were to finally unify ref-filter and cat-file (and\neven --pretty=format) then it would probably worth doing this kind of\npre-parsing.\n\n---\ndiff --git a/builtin/cat-file.c b/builtin/cat-file.c\nindex 9cc7ec7a6f..da6ecc61f9 100644\n--- a/builtin/cat-file.c\n+++ b/builtin/cat-file.c\n@@ -321,7 +321,17 @@ struct expand_data {\n #define EXPAND_DATA_INIT  { .mode = S_IFINVALID }\n \n struct format_item {\n-\tvoid (*add)(struct format_item *item, struct strbuf *sb, struct expand_data *data);\n+\tenum {\n+\t\tFORMAT_TYPE_END = 0,\n+\t\tFORMAT_TYPE_LITERAL,\n+\t\tFORMAT_TYPE_OBJECTNAME,\n+\t\tFORMAT_TYPE_OBJECTTYPE,\n+\t\tFORMAT_TYPE_OBJECTSIZE,\n+\t\tFORMAT_TYPE_OBJECTSIZE_DISK,\n+\t\tFORMAT_TYPE_REST,\n+\t\tFORMAT_TYPE_DELTABASE,\n+\t\tFORMAT_TYPE_OBJECTMODE,\n+\t} type;\n \tunion {\n \t\tstruct {\n \t\t\tconst char *p;\n@@ -336,55 +346,6 @@ struct format_item {\n \t */\n };\n \n-static void objectname_add(struct format_item *item UNUSED,\n-\t\t\t   struct strbuf *sb, struct expand_data *data)\n-{\n-\tstrbuf_add_oid_hex(sb, &data->oid);\n-}\n-\n-static void objecttype_add(struct format_item *item UNUSED,\n-\t\t\t   struct strbuf *sb, struct expand_data *data)\n-{\n-\tstrbuf_addstr(sb, type_name(data->type));\n-}\n-\n-static void objectsize_add(struct format_item *item UNUSED,\n-\t\t\t   struct strbuf *sb, struct expand_data *data)\n-{\n-\tstrbuf_add_uint(sb, data->size);\n-}\n-\n-static void objectsize_disk_add(struct format_item *item UNUSED,\n-\t\t\t\tstruct strbuf *sb, struct expand_data *data)\n-{\n-\tstrbuf_add_uint(sb, data->disk_size);\n-}\n-\n-static void rest_add(struct format_item *item UNUSED,\n-\t\t     struct strbuf *sb, struct expand_data *data)\n-{\n-\tstrbuf_addstr(sb, data->rest);\n-}\n-\n-static void deltabase_add(struct format_item *item UNUSED,\n-\t\t\t  struct strbuf *sb, struct expand_data *data)\n-{\n-\tstrbuf_add_oid_hex(sb, &data->delta_base_oid);\n-}\n-\n-static void objectmode_add(struct format_item *item UNUSED,\n-\t\t\t   struct strbuf *sb, struct expand_data *data)\n-{\n-\tif (data->mode != S_IFINVALID)\n-\t\tstrbuf_addf(sb, \"%06o\", data->mode);\n-}\n-\n-static void literal_add(struct format_item *item,\n-\t\t\tstruct strbuf *sb, struct expand_data *data UNUSED)\n-{\n-\tstrbuf_add(sb, item->u.literal.p, item->u.literal.len);\n-}\n-\n static int is_atom(const char *atom, const char *s, int slen)\n {\n \tint alen = strlen(atom);\n@@ -395,24 +356,24 @@ static int parse_atom(struct format_item *fmt, const char *atom, int len,\n \t\t      struct expand_data *data)\n {\n \tif (is_atom(\"objectname\", atom, len)) {\n-\t\tfmt->add = objectname_add;\n+\t\tfmt->type = FORMAT_TYPE_OBJECTNAME;\n \t} else if (is_atom(\"objecttype\", atom, len)) {\n \t\tdata->info.typep = &data->type;\n-\t\tfmt->add = objecttype_add;\n+\t\tfmt->type = FORMAT_TYPE_OBJECTTYPE;\n \t} else if (is_atom(\"objectsize\", atom, len)) {\n \t\tdata->info.sizep = &data->size;\n-\t\tfmt->add = objectsize_add;\n+\t\tfmt->type = FORMAT_TYPE_OBJECTSIZE;\n \t} else if (is_atom(\"objectsize:disk\", atom, len)) {\n \t\tdata->info.disk_sizep = &data->disk_size;\n-\t\tfmt->add = objectsize_disk_add;\n+\t\tfmt->type = FORMAT_TYPE_OBJECTSIZE_DISK;\n \t} else if (is_atom(\"rest\", atom, len)) {\n \t\tdata->split_on_whitespace = 1;\n-\t\tfmt->add = rest_add;\n+\t\tfmt->type = FORMAT_TYPE_REST;\n \t} else if (is_atom(\"deltabase\", atom, len)) {\n \t\tdata->info.delta_base_oid = &data->delta_base_oid;\n-\t\tfmt->add = deltabase_add;\n+\t\tfmt->type = FORMAT_TYPE_DELTABASE;\n \t} else if (is_atom(\"objectmode\", atom, len)) {\n-\t\tfmt->add = objectmode_add;\n+\t\tfmt->type = FORMAT_TYPE_OBJECTMODE;\n \t} else\n \t\treturn 0;\n \treturn 1;\n@@ -430,7 +391,7 @@ static struct format_item *parse_format(const char *start,\n \n \t\tif (percent != start) {\n \t\t\tALLOC_GROW(ret, nr + 1, alloc);\n-\t\t\tret[nr].add = literal_add;\n+\t\t\tret[nr].type = FORMAT_TYPE_LITERAL;\n \t\t\tret[nr].u.literal.p = start;\n \t\t\tret[nr].u.literal.len = percent - start;\n \t\t\tnr++;\n@@ -443,7 +404,7 @@ static struct format_item *parse_format(const char *start,\n \n \t\tALLOC_GROW(ret, nr + 1, alloc);\n \t\tif (skip_prefix(start, \"%\", &start) || *start != '(') {\n-\t\t\tret[nr].add = literal_add;\n+\t\t\tret[nr].type = FORMAT_TYPE_LITERAL;\n \t\t\tret[nr].u.literal.p = \"%\";\n \t\t\tret[nr].u.literal.len = 1;\n \t\t} else if ((end = strchr(start + 1, ')')) &&\n@@ -456,16 +417,44 @@ static struct format_item *parse_format(const char *start,\n \t}\n \n \tALLOC_GROW(ret, nr + 1, alloc);\n-\tret[nr].add = NULL;\n+\tret[nr].type = FORMAT_TYPE_END;\n \n \treturn ret;\n }\n \n static void expand_format(struct strbuf *sb, struct format_item *fmt,\n \t\t\t  struct expand_data *data)\n {\n-\tfor (; fmt->add; fmt++)\n-\t\tfmt->add(fmt, sb, data);\n+\tfor (; fmt->type; fmt++)\n+\t\tswitch (fmt->type) {\n+\t\tcase FORMAT_TYPE_END:\n+\t\t\tBUG(\"we should have already left the loop!\");\n+\t\t\tbreak;\n+\t\tcase FORMAT_TYPE_OBJECTNAME:\n+\t\t\tstrbuf_add_oid_hex(sb, &data->oid);\n+\t\t\tbreak;\n+\t\tcase FORMAT_TYPE_OBJECTTYPE:\n+\t\t\tstrbuf_addstr(sb, type_name(data->type));\n+\t\t\tbreak;\n+\t\tcase FORMAT_TYPE_OBJECTSIZE:\n+\t\t\tstrbuf_add_uint(sb, data->size);\n+\t\t\tbreak;\n+\t\tcase FORMAT_TYPE_OBJECTSIZE_DISK:\n+\t\t\tstrbuf_add_uint(sb, data->disk_size);\n+\t\t\tbreak;\n+\t\tcase FORMAT_TYPE_REST:\n+\t\t\tstrbuf_addstr(sb, data->rest);\n+\t\t\tbreak;\n+\t\tcase FORMAT_TYPE_DELTABASE:\n+\t\t\tstrbuf_add_oid_hex(sb, &data->delta_base_oid);\n+\t\t\tbreak;\n+\t\tcase FORMAT_TYPE_OBJECTMODE:\n+\t\t\tif (data->mode != S_IFINVALID)\n+\t\t\t\tstrbuf_addf(sb, \"%06o\", data->mode);\n+\t\t\tbreak;\n+\t\tcase FORMAT_TYPE_LITERAL:\n+\t\t\tstrbuf_add(sb, fmt->u.literal.p, fmt->u.literal.len);\n+\t\t}\n }\n \n static void batch_write(struct batch_options *opt, const void *data, int len)\n"},{"id":"545615","messageId":"df933ffa-1be2-4401-a4ac-9d72c9c4cdcc@web.de","threadId":"65809","inReplyTo":"20260615165326.GA91269@coredump.intra.peff.net","subject":"Re: [PATCH] cat-file: speed up default format","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2026-06-15T21:53:10Z","receivedAt":"2026-06-15T21:53:11Z","isPatch":true,"body":"On 6/15/26 6:53 PM, Jeff King wrote:\n> \n> +static void rest_add(struct format_item *item UNUSED,\n> +\t\t     struct strbuf *sb, struct expand_data *data)\n> +{\n> +\tstrbuf_addstr(sb, data->rest);\n> +}\n\n>  \t} else if (is_atom(\"rest\", atom, len)) {\n> -\t\tif (data->mark_query)\n> -\t\t\tdata->split_on_whitespace = 1;\n> -\t\telse if (data->rest)\n\nThis removes support for rest being NULL, breaking t1006.381.\n\n> -\t\t\tstrbuf_addstr(sb, data->rest);\n> +\t\tdata->split_on_whitespace = 1;\n> +\t\tfmt->add = rest_add;\nRené\n\n"},{"id":"545616","messageId":"10a33614-837f-4166-aa30-6de28b052692@web.de","threadId":"65809","inReplyTo":"20260615170652.GB91269@coredump.intra.peff.net","subject":"Re: [PATCH] cat-file: speed up default format","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2026-06-15T21:53:07Z","receivedAt":"2026-06-15T21:53:19Z","isPatch":true,"body":"On 6/15/26 7:06 PM, Jeff King wrote:\n> On Mon, Jun 15, 2026 at 12:53:26PM -0400, Jeff King wrote:\n> \n>> It uses per-atom callback functions which is nice and clean, though we\n>> might be able to do even better with a big ugly switch() statement.\n> \n> Being the curious sort, I swapped it out for a big switch statement.\n> Patch below, but it does not seem to be any faster.\n> \n> So the bottom line is I think you could gain a little bit of performance\n> by pre-parsing (versus strbuf_expand() on each object). Around 3% for\n> something that actually looks at the objects, though more like 15% if\n> for just dumping the objectnames.\n> \n> IMHO that is probably not worth it for a custom parsing system just for\n> cat-file.  But if we were to finally unify ref-filter and cat-file (and\n> even --pretty=format) then it would probably worth doing this kind of\n> pre-parsing.\nIt could be worth it for cat-file alone if we find the right balance, as\nit already does do a separate parsing step, but that is awkward with its\nmark_query checks all over the place and remembers only object property\nrequirements and no other format string details.\n\nMaking the opcodes small should be beneficial.  We need only a handful\nof them, so a byte each should suffice.  We can use a strbuf for that.\n\nWe can also store literal characters in there.  An opcode plus with a\npayload char incurs an overhead of 50%, which sounds high, but at least\nthe default format only has two of them and it's much better than\nstoring pointer plus size for an overhead of more than 90% in case of a\nsingle char.\n\nThat gets us closer to native speed, at least on an Apple M1:\n\nBenchmark 1: ./git_fp cat-file --batch-all-objects --batch-check='%(objectname)-%(objecttype)-%(objectsize)'\n  Time (mean ± σ):     992.7 ms ±   3.2 ms    [User: 967.5 ms, System: 23.8 ms]\n  Range (min … max):   990.1 ms … 1000.7 ms    10 runs\n\nBenchmark 2: ./git_switch cat-file --batch-all-objects --batch-check='%(objectname)-%(objecttype)-%(objectsize)'\n  Time (mean ± σ):     991.8 ms ±   1.6 ms    [User: 967.0 ms, System: 23.3 ms]\n  Range (min … max):   989.3 ms … 994.4 ms    10 runs\n\nBenchmark 3: ./git cat-file --batch-all-objects --batch-check='%(objectname)-%(objecttype)-%(objectsize)'\n  Time (mean ± σ):     985.8 ms ±   2.9 ms    [User: 960.5 ms, System: 23.6 ms]\n  Range (min … max):   982.9 ms … 993.0 ms    10 runs\n\nBenchmark 4: ./git cat-file --batch-all-objects --batch-check='%(objectname) %(objecttype) %(objectsize)'\n  Time (mean ± σ):     982.1 ms ±   3.2 ms    [User: 956.7 ms, System: 23.6 ms]\n  Range (min … max):   979.2 ms … 989.2 ms    10 runs\n\nSummary\n  ./git cat-file --batch-all-objects --batch-check='%(objectname) %(objecttype) %(objectsize)' ran\n    1.00 ± 0.00 times faster than ./git cat-file --batch-all-objects --batch-check='%(objectname)-%(objecttype)-%(objectsize)'\n    1.01 ± 0.00 times faster than ./git_switch cat-file --batch-all-objects --batch-check='%(objectname)-%(objecttype)-%(objectsize)'\n    1.01 ± 0.00 times faster than ./git_fp cat-file --batch-all-objects --batch-check='%(objectname)-%(objecttype)-%(objectsize)'\n\n\nA Ryzen laptop gives me noisy numbers that seem to suggest your\nswitch-based code already won, but the more compact representation is at\nleast not worse:\n\nBenchmark 1: ./git_fp cat-file --batch-all-objects --batch-check='%(objectname)-%(objecttype)-%(objectsize)'\n  Time (mean ± σ):     397.5 ms ±   8.0 ms    [User: 326.9 ms, System: 39.4 ms]\n  Range (min … max):   388.1 ms … 410.0 ms    10 runs\n\nBenchmark 2: ./git_switch cat-file --batch-all-objects --batch-check='%(objectname)-%(objecttype)-%(objectsize)'\n  Time (mean ± σ):     388.2 ms ±   4.2 ms    [User: 318.2 ms, System: 39.2 ms]\n  Range (min … max):   382.8 ms … 395.7 ms    10 runs\n\nBenchmark 3: ./git cat-file --batch-all-objects --batch-check='%(objectname)-%(objecttype)-%(objectsize)'\n  Time (mean ± σ):     385.5 ms ±   5.7 ms    [User: 311.2 ms, System: 43.2 ms]\n  Range (min … max):   377.0 ms … 392.9 ms    10 runs\n\nBenchmark 4: ./git cat-file --batch-all-objects --batch-check='%(objectname) %(objecttype) %(objectsize)'\n  Time (mean ± σ):     397.5 ms ±   8.4 ms    [User: 321.9 ms, System: 45.2 ms]\n  Range (min … max):   382.1 ms … 406.5 ms    10 runs\n\nSummary\n  ./git cat-file --batch-all-objects --batch-check='%(objectname)-%(objecttype)-%(objectsize)' ran\n    1.01 ± 0.02 times faster than ./git_switch cat-file --batch-all-objects --batch-check='%(objectname)-%(objecttype)-%(objectsize)'\n    1.03 ± 0.03 times faster than ./git_fp cat-file --batch-all-objects --batch-check='%(objectname)-%(objecttype)-%(objectsize)'\n    1.03 ± 0.03 times faster than ./git cat-file --batch-all-objects --batch-check='%(objectname) %(objecttype) %(objectsize)'\n\nRené\n\n\ndiff --git a/builtin/cat-file.c b/builtin/cat-file.c\nindex 0d1998784c..5667a13e93 100644\n--- a/builtin/cat-file.c\n+++ b/builtin/cat-file.c\n@@ -36,8 +36,6 @@ enum batch_mode {\n \tBATCH_MODE_QUEUE_AND_DISPATCH,\n };\n \n-struct format_item;\n-\n struct batch_options {\n \tstruct list_objects_filter_options objects_filter;\n \tint enabled;\n@@ -50,7 +48,7 @@ struct batch_options {\n \tchar input_delim;\n \tchar output_delim;\n \tconst char *format;\n-\tstruct format_item *parsed_format;\n+\tstruct strbuf parsed_format;\n };\n \n static const char *force_path;\n@@ -320,30 +318,16 @@ struct expand_data {\n };\n #define EXPAND_DATA_INIT  { .mode = S_IFINVALID }\n \n-struct format_item {\n-\tenum {\n-\t\tFORMAT_TYPE_END = 0,\n-\t\tFORMAT_TYPE_LITERAL,\n-\t\tFORMAT_TYPE_OBJECTNAME,\n-\t\tFORMAT_TYPE_OBJECTTYPE,\n-\t\tFORMAT_TYPE_OBJECTSIZE,\n-\t\tFORMAT_TYPE_OBJECTSIZE_DISK,\n-\t\tFORMAT_TYPE_REST,\n-\t\tFORMAT_TYPE_DELTABASE,\n-\t\tFORMAT_TYPE_OBJECTMODE,\n-\t} type;\n-\tunion {\n-\t\tstruct {\n-\t\t\tconst char *p;\n-\t\t\tsize_t len;\n-\t\t} literal;\n-\t} u;\n-\t/*\n-\t * We could make a true tree here with child/next pointers, which would\n-\t * be necessary if we had recursive formats, like %(if). But for our\n-\t * simple formats for now it is enough to have a linear set of items,\n-\t * so we'll just allocate an array and terminate it with a NULL entry.\n-\t */\n+\n+enum item_type {\n+\tFORMAT_TYPE_LITERAL,\n+\tFORMAT_TYPE_OBJECTNAME,\n+\tFORMAT_TYPE_OBJECTTYPE,\n+\tFORMAT_TYPE_OBJECTSIZE,\n+\tFORMAT_TYPE_OBJECTSIZE_DISK,\n+\tFORMAT_TYPE_REST,\n+\tFORMAT_TYPE_DELTABASE,\n+\tFORMAT_TYPE_OBJECTMODE,\n };\n \n static int is_atom(const char *atom, const char *s, int slen)\n@@ -352,84 +336,66 @@ static int is_atom(const char *atom, const char *s, int slen)\n \treturn alen == slen && !memcmp(atom, s, alen);\n }\n \n-static int parse_atom(struct format_item *fmt, const char *atom, int len,\n+static int parse_atom(struct strbuf *parsed_format, const char *atom, int len,\n \t\t      struct expand_data *data)\n {\n \tif (is_atom(\"objectname\", atom, len)) {\n-\t\tfmt->type = FORMAT_TYPE_OBJECTNAME;\n+\t\tstrbuf_addch(parsed_format, FORMAT_TYPE_OBJECTNAME);\n \t} else if (is_atom(\"objecttype\", atom, len)) {\n \t\tdata->info.typep = &data->type;\n-\t\tfmt->type = FORMAT_TYPE_OBJECTTYPE;\n+\t\tstrbuf_addch(parsed_format, FORMAT_TYPE_OBJECTTYPE);\n \t} else if (is_atom(\"objectsize\", atom, len)) {\n \t\tdata->info.sizep = &data->size;\n-\t\tfmt->type = FORMAT_TYPE_OBJECTSIZE;\n+\t\tstrbuf_addch(parsed_format, FORMAT_TYPE_OBJECTSIZE);\n \t} else if (is_atom(\"objectsize:disk\", atom, len)) {\n \t\tdata->info.disk_sizep = &data->disk_size;\n-\t\tfmt->type = FORMAT_TYPE_OBJECTSIZE_DISK;\n+\t\tstrbuf_addch(parsed_format, FORMAT_TYPE_OBJECTSIZE_DISK);\n \t} else if (is_atom(\"rest\", atom, len)) {\n \t\tdata->split_on_whitespace = 1;\n-\t\tfmt->type = FORMAT_TYPE_REST;\n+\t\tstrbuf_addch(parsed_format, FORMAT_TYPE_REST);\n \t} else if (is_atom(\"deltabase\", atom, len)) {\n \t\tdata->info.delta_base_oid = &data->delta_base_oid;\n-\t\tfmt->type = FORMAT_TYPE_DELTABASE;\n+\t\tstrbuf_addch(parsed_format, FORMAT_TYPE_DELTABASE);\n \t} else if (is_atom(\"objectmode\", atom, len)) {\n-\t\tfmt->type = FORMAT_TYPE_OBJECTMODE;\n+\t\tstrbuf_addch(parsed_format, FORMAT_TYPE_OBJECTMODE);\n \t} else\n \t\treturn 0;\n \treturn 1;\n }\n \n-static struct format_item *parse_format(const char *start,\n-\t\t\t\t\tstruct expand_data *data)\n+static void parse_format(struct strbuf *parsed_format,\n+\t\t\t const char *start, struct expand_data *data)\n {\n-\tstruct format_item *ret = NULL;\n-\tsize_t nr = 0, alloc = 0;\n-\n \twhile (1) {\n-\t\tconst char *percent = strchrnul(start, '%');\n \t\tconst char *end;\n \n-\t\tif (percent != start) {\n-\t\t\tALLOC_GROW(ret, nr + 1, alloc);\n-\t\t\tret[nr].type = FORMAT_TYPE_LITERAL;\n-\t\t\tret[nr].u.literal.p = start;\n-\t\t\tret[nr].u.literal.len = percent - start;\n-\t\t\tnr++;\n+\t\twhile (*start && *start != '%') {\n+\t\t\tstrbuf_addch(parsed_format, FORMAT_TYPE_LITERAL);\n+\t\t\tstrbuf_addch(parsed_format, *start++);\n \t\t}\n \n-\t\tif (!*percent)\n+\t\tif (!*start)\n \t\t\tbreak;\n \n-\t\tstart = percent + 1;\n+\t\tstart++;\n \n-\t\tALLOC_GROW(ret, nr + 1, alloc);\n \t\tif (skip_prefix(start, \"%\", &start) || *start != '(') {\n-\t\t\tret[nr].type = FORMAT_TYPE_LITERAL;\n-\t\t\tret[nr].u.literal.p = \"%\";\n-\t\t\tret[nr].u.literal.len = 1;\n+\t\t\tstrbuf_addch(parsed_format, FORMAT_TYPE_LITERAL);\n+\t\t\tstrbuf_addch(parsed_format, '%');\n \t\t} else if ((end = strchr(start + 1, ')')) &&\n-\t\t\t   parse_atom(&ret[nr], start + 1, end - start - 1, data)) {\n+\t\t\t   parse_atom(parsed_format, start + 1, end - start - 1, data)) {\n \t\t\tstart = end + 1;\n \t\t} else {\n \t\t\tstrbuf_expand_bad_format(start, \"cat-file\");\n \t\t}\n-\t\tnr++;\n \t}\n-\n-\tALLOC_GROW(ret, nr + 1, alloc);\n-\tret[nr].type = FORMAT_TYPE_END;\n-\n-\treturn ret;\n }\n \n-static void expand_format(struct strbuf *sb, struct format_item *fmt,\n+static void expand_format(struct strbuf *sb, struct strbuf *parsed_format,\n \t\t\t  struct expand_data *data)\n {\n-\tfor (; fmt->type; fmt++)\n-\t\tswitch (fmt->type) {\n-\t\tcase FORMAT_TYPE_END:\n-\t\t\tBUG(\"we should have already left the loop!\");\n-\t\t\tbreak;\n+\tfor (size_t i = 0; i < parsed_format->len; i++)\n+\t\tswitch (parsed_format->buf[i]) {\n \t\tcase FORMAT_TYPE_OBJECTNAME:\n \t\t\tstrbuf_add_oid_hex(sb, &data->oid);\n \t\t\tbreak;\n@@ -453,7 +419,7 @@ static void expand_format(struct strbuf *sb, struct format_item *fmt,\n \t\t\t\tstrbuf_addf(sb, \"%06o\", data->mode);\n \t\t\tbreak;\n \t\tcase FORMAT_TYPE_LITERAL:\n-\t\t\tstrbuf_add(sb, fmt->u.literal.p, fmt->u.literal.len);\n+\t\t\tstrbuf_addch(sb, parsed_format->buf[++i]);\n \t\t}\n }\n \n@@ -641,7 +607,7 @@ static void batch_object_write(const char *obj_name,\n \tif (!opt->format) {\n \t\tprint_default_format(scratch, data, opt);\n \t} else {\n-\t\texpand_format(scratch, opt->parsed_format, data);\n+\t\texpand_format(scratch, &opt->parsed_format, data);\n \t\tstrbuf_addch(scratch, opt->output_delim);\n \t}\n \n@@ -1010,9 +976,8 @@ static int batch_objects(struct batch_options *opt)\n \tint save_warning;\n \tint retval = 0;\n \n-\topt->parsed_format = parse_format(opt->format ?\n-\t\t\t\t\t  opt->format : DEFAULT_FORMAT,\n-\t\t\t\t\t  &data);\n+\tparse_format(&opt->parsed_format,\n+\t\t     opt->format ? opt->format : DEFAULT_FORMAT, &data);\n \tif (opt->transform_mode)\n \t\tdata.split_on_whitespace = 1;\n \n@@ -1152,6 +1117,7 @@ int cmd_cat_file(int argc,\n \tconst char *exp_type = NULL, *obj_name = NULL;\n \tstruct batch_options batch = {\n \t\t.objects_filter = LIST_OBJECTS_FILTER_INIT,\n+\t\t.parsed_format = STRBUF_INIT,\n \t};\n \tint unknown_type = 0;\n \tint input_nul_terminated = 0;\n\n"},{"id":"545647","messageId":"20260616111237.GA687438@coredump.intra.peff.net","threadId":"65809","inReplyTo":"10a33614-837f-4166-aa30-6de28b052692@web.de","subject":"Re: [PATCH] cat-file: speed up default format","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-06-16T11:12:37Z","receivedAt":"2026-06-16T11:12:38Z","isPatch":true,"body":"On Mon, Jun 15, 2026 at 11:53:07PM +0200, René Scharfe wrote:\n\n> > IMHO that is probably not worth it for a custom parsing system just for\n> > cat-file.  But if we were to finally unify ref-filter and cat-file (and\n> > even --pretty=format) then it would probably worth doing this kind of\n> > pre-parsing.\n> It could be worth it for cat-file alone if we find the right balance, as\n> it already does do a separate parsing step, but that is awkward with its\n> mark_query checks all over the place and remembers only object property\n> requirements and no other format string details.\n\nYeah, getting rid of the mark_query pass was a nice side effect of\nhaving a true parse step.\n\n> Making the opcodes small should be beneficial.  We need only a handful\n> of them, so a byte each should suffice.  We can use a strbuf for that.\n> \n> We can also store literal characters in there.  An opcode plus with a\n> payload char incurs an overhead of 50%, which sounds high, but at least\n> the default format only has two of them and it's much better than\n> storing pointer plus size for an overhead of more than 90% in case of a\n> single char.\n\nTrue, and it's a size win if the literal portions tend to be small\n(fewer than 15 bytes). You do lose out on the ability to strbuf_add()\nthem in one go, though. So lots more strbuf_grow() checks, etc. If you\nreally wanted to get fancy, you could follow the opcode with a length\nrepresented as a variable-sized integer, followed by the literal bytes.\n\nI'm not sure that Git's formatting code needs to squeeze out quite that\nmuch performance, though.\n\n> That gets us closer to native speed, at least on an Apple M1:\n> \n> Benchmark 1: ./git_fp cat-file --batch-all-objects --batch-check='%(objectname)-%(objecttype)-%(objectsize)'\n>   Time (mean ± σ):     992.7 ms ±   3.2 ms    [User: 967.5 ms, System: 23.8 ms]\n>   Range (min … max):   990.1 ms … 1000.7 ms    10 runs\n> \n> Benchmark 2: ./git_switch cat-file --batch-all-objects --batch-check='%(objectname)-%(objecttype)-%(objectsize)'\n>   Time (mean ± σ):     991.8 ms ±   1.6 ms    [User: 967.0 ms, System: 23.3 ms]\n>   Range (min … max):   989.3 ms … 994.4 ms    10 runs\n> \n> Benchmark 3: ./git cat-file --batch-all-objects --batch-check='%(objectname)-%(objecttype)-%(objectsize)'\n>   Time (mean ± σ):     985.8 ms ±   2.9 ms    [User: 960.5 ms, System: 23.6 ms]\n>   Range (min … max):   982.9 ms … 993.0 ms    10 runs\n> \n> Benchmark 4: ./git cat-file --batch-all-objects --batch-check='%(objectname) %(objecttype) %(objectsize)'\n>   Time (mean ± σ):     982.1 ms ±   3.2 ms    [User: 956.7 ms, System: 23.6 ms]\n>   Range (min … max):   979.2 ms … 989.2 ms    10 runs\n\nOK, so we managed another 1%. But I'm skeptical that this linear opcode\ntechnique is where we want to go in the long run, if we're ever going to\nunify formatters.\n\nOne, for more advanced features like %(if) we'd want to support some\nnotion of hierarchy and recursion. We have to speculatively format the\ninner part and see if it is empty.\n\nThough I guess that is possible with a linearized set of opcodes. If you\nturn \"%(if)%(HEAD)%(then)*%(end)\" into:\n\n  FMT_IF\n  FMT_HEAD\n  FMT_THEN\n  FMT_LITERAL\n  *\n  FMT_END\n\nthen I guess you just start a sub-execution of the opcodes after FMT_IF\nand tell it to stop when it sees FMT_THEN. It does mean that the opcodes\nthemselves need to control the program counter, rather than the executor\nblindly walking along the opcodes and asking them to put stuff in the\noutput. Whereas I think if the parser builds a tree of structs then this\nfalls out pretty naturally.\n\nThe second thing is that many of the ref-filter atoms have options, and\nthose options have to be represented in the opcodes. That works\nnaturally if each opcode gets its own struct (either with a big union,\nor true polymorphism). But representing \"%(describe:match=versions/v*)\"\nin opcodes sounds gross. Now you need opcodes to represent the options\n(and maybe \"no more options\"), and some way of encoding arbitrary input\nfor those option arguments.\n\n-Peff\n"},{"id":"545648","messageId":"20260616111534.GB687438@coredump.intra.peff.net","threadId":"65809","inReplyTo":"df933ffa-1be2-4401-a4ac-9d72c9c4cdcc@web.de","subject":"Re: [PATCH] cat-file: speed up default format","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-06-16T11:15:34Z","receivedAt":"2026-06-16T11:15:35Z","isPatch":true,"body":"On Mon, Jun 15, 2026 at 11:53:10PM +0200, René Scharfe wrote:\n\n> >  \t} else if (is_atom(\"rest\", atom, len)) {\n> > -\t\tif (data->mark_query)\n> > -\t\t\tdata->split_on_whitespace = 1;\n> > -\t\telse if (data->rest)\n> \n> This removes support for rest being NULL, breaking t1006.381.\n\nYup. I did say \"only lightly tested\". ;)\n\nThe fix is obviously just:\n\ndiff --git a/builtin/cat-file.c b/builtin/cat-file.c\nindex 9cc7ec7a6f..370ca6d771 100644\n--- a/builtin/cat-file.c\n+++ b/builtin/cat-file.c\n@@ -363,7 +363,8 @@ static void objectsize_disk_add(struct format_item *item UNUSED,\n static void rest_add(struct format_item *item UNUSED,\n \t\t     struct strbuf *sb, struct expand_data *data)\n {\n-\tstrbuf_addstr(sb, data->rest);\n+\tif (data->rest)\n+\t\tstrbuf_addstr(sb, data->rest);\n }\n \n static void deltabase_add(struct format_item *item UNUSED,\n\nI think perhaps this error shows that the mark_query thing in the\nexisting code obfuscates the logic a bit.\n\n-Peff\n"},{"id":"545698","messageId":"47e3cf16-217e-45d4-91e2-5a1abb4ee49e@web.de","threadId":"65809","inReplyTo":"20260616111237.GA687438@coredump.intra.peff.net","subject":"Re: [PATCH] cat-file: speed up default format","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2026-06-16T22:14:37Z","receivedAt":"2026-06-16T22:14:48Z","isPatch":true,"body":"On 6/16/26 1:12 PM, Jeff King wrote:\n> On Mon, Jun 15, 2026 at 11:53:07PM +0200, René Scharfe wrote:\n> \n>> We can also store literal characters in there.  An opcode plus with a\n>> payload char incurs an overhead of 50%, which sounds high, but at least\n>> the default format only has two of them and it's much better than\n>> storing pointer plus size for an overhead of more than 90% in case of a\n>> single char.\n> \n> True, and it's a size win if the literal portions tend to be small\n> (fewer than 15 bytes). You do lose out on the ability to strbuf_add()\n> them in one go, though. So lots more strbuf_grow() checks, etc. If you\n> really wanted to get fancy, you could follow the opcode with a length\n> represented as a variable-sized integer, followed by the literal bytes.\n\nOr an opcode that shovels a fixed-size string.  Depends on how much\nliteral text people include in their formats.\n\n> I'm not sure that Git's formatting code needs to squeeze out quite that\n> much performance, though.\n\nGood point.  Near-native performance would be necessary to make peephole\noptimizations like the special handling of the default format\nunnecessary, which I understand exists to speed up Gitaly [1], but I\nguess most users don't have such high demands.  And there's no point in\nremoving a few lines of duplicate code if the necessary machinery adds a\nlot of complexity.  Though the code discussed so far was not too crazy\nIMHO.\n\n[1] https://gitlab.com/gitlab-org/gitaly/-/blob/master/internal/git/gitpipe/catfile_info.go\n\n> OK, so we managed another 1%. But I'm skeptical that this linear opcode\n> technique is where we want to go in the long run, if we're ever going to\n> unify formatters.\n\nAgreed.\n\nRené\n\n"},{"id":"545699","messageId":"xmqqo6ha15zv.fsf@gitster.g","threadId":"65809","inReplyTo":"47e3cf16-217e-45d4-91e2-5a1abb4ee49e@web.de","subject":"Re: [PATCH] cat-file: speed up default format","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-06-16T22:21:24Z","receivedAt":"2026-06-16T22:21:27Z","isPatch":true,"body":"René Scharfe <l.s.r@web.de> writes:\n\n> On 6/16/26 1:12 PM, Jeff King wrote:\n>> On Mon, Jun 15, 2026 at 11:53:07PM +0200, René Scharfe wrote:\n>> \n>>> We can also store literal characters in there.  An opcode plus with a\n>>> payload char incurs an overhead of 50%, which sounds high, but at least\n>>> the default format only has two of them and it's much better than\n>>> storing pointer plus size for an overhead of more than 90% in case of a\n>>> single char.\n>> \n>> True, and it's a size win if the literal portions tend to be small\n>> (fewer than 15 bytes). You do lose out on the ability to strbuf_add()\n>> them in one go, though. So lots more strbuf_grow() checks, etc. If you\n>> really wanted to get fancy, you could follow the opcode with a length\n>> represented as a variable-sized integer, followed by the literal bytes.\n>\n> Or an opcode that shovels a fixed-size string.  Depends on how much\n> literal text people include in their formats.\n>\n>> I'm not sure that Git's formatting code needs to squeeze out quite that\n>> much performance, though.\n>\n> Good point.  Near-native performance would be necessary to make peephole\n> optimizations like the special handling of the default format\n> unnecessary, which I understand exists to speed up Gitaly [1], but I\n> guess most users don't have such high demands.  And there's no point in\n> removing a few lines of duplicate code if the necessary machinery adds a\n> lot of complexity.  Though the code discussed so far was not too crazy\n> IMHO.\n>\n> [1] https://gitlab.com/gitlab-org/gitaly/-/blob/master/internal/git/gitpipe/catfile_info.go\n>\n>> OK, so we managed another 1%. But I'm skeptical that this linear opcode\n>> technique is where we want to go in the long run, if we're ever going to\n>> unify formatters.\n>\n> Agreed.\n>\n> René\n\nObviously I agree with the conclusion, but it was fun to watch cute\nexperiments from the sidelines.  Thanks for entertainment ;-)\n"}]}