{"thread":{"id":"55517","subject":"[PATCH 0/2] [GSOC] ref-filter: reuse output buffer","startedAt":"2021-04-19T11:28:52Z","lastAt":"2021-04-20T16:52:24Z","messageCount":9,"participants":["ZheNing Hu via GitGitGadget","Junio C Hamano","ZheNing Hu"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"422264","messageId":"pull.935.git.1618831726.gitgitgadget@gmail.com","threadId":"55517","inReplyTo":null,"subject":"[PATCH 0/2] [GSOC] ref-filter: reuse output buffer","fromName":"ZheNing Hu via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-04-19T11:28:43Z","receivedAt":"2021-04-19T11:28:52Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":" * Firstly, inlining show_ref_array_item().\n * Secondly, git for-each-ref reuse final buf for all refs output, the\n performance is slightly improved, this optimization is also applied to\n git tag -l and git branch -l.\n\nThanks.\n\nZheNing Hu (2):\n [GSOC] ref-filter: get rid of show_ref_array_item\n [GSOC] ref-filter: reuse output buffer\n\n builtin/branch.c | 9 +++++----\n builtin/for-each-ref.c | 14 ++++++++++++--\n builtin/tag.c | 14 ++++++++++++--\n ref-filter.c | 25 ++++++++++---------------\n ref-filter.h | 2 --\n 5 files changed, 39 insertions(+), 25 deletions(-)\n\n\nbase-commit: 2e36527f23b7f6ae15e6f21ac3b08bf3fed6ee48\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-935%2Fadlternative%2Fref-filter-opt-reuse-buf-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-935/adlternative/ref-filter-opt-reuse-buf-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/935\n-- \ngitgitgadget\n"},{"id":"422265","messageId":"ec98f2177d905cc9b3c61e15ddb756243536c125.1618831726.git.gitgitgadget@gmail.com","threadId":"55517","inReplyTo":"pull.935.git.1618831726.gitgitgadget@gmail.com","subject":"[PATCH 1/2] [GSOC] ref-filter: get rid of show_ref_array_item","fromName":"ZheNing Hu via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-04-19T11:28:44Z","receivedAt":"2021-04-19T11:28:52Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"From: ZheNing Hu \n\nInlining the exported function `show_ref_array_item()`\nwhich is not providing the right level of abstraction,\nsimplifies the API and can unlock improvements at the\nformer call sites.\n\nHelped-by: René Scharfe \nSigned-off-by: ZheNing Hu \n---\n builtin/for-each-ref.c | 14 ++++++++++++--\n builtin/tag.c | 14 ++++++++++++--\n ref-filter.c | 25 ++++++++++---------------\n ref-filter.h | 2 --\n 4 files changed, 34 insertions(+), 21 deletions(-)\n\ndiff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c\nindex cb9c81a04606..8520008604e3 100644\n--- a/builtin/for-each-ref.c\n+++ b/builtin/for-each-ref.c\n@@ -80,8 +80,18 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)\n \n \tif (!maxcount || array.nr < maxcount)\n \t\tmaxcount = array.nr;\n-\tfor (i = 0; i < maxcount; i++)\n-\t\tshow_ref_array_item(array.items[i], &format);\n+\tfor (i = 0; i < maxcount; i++) {\n+\t\tstruct strbuf output = STRBUF_INIT;\n+\t\tstruct strbuf err = STRBUF_INIT;\n+\n+\t\tif (format_ref_array_item(array.items[i], &format, &output, &err))\n+\t\t\tdie(\"%s\", err.buf);\n+\t\tfwrite(output.buf, 1, output.len, stdout);\n+\t\tputchar('\\n');\n+\n+\t\tstrbuf_release(&err);\n+\t\tstrbuf_release(&output);\n+\t}\n \tref_array_clear(&array);\n \treturn 0;\n }\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex d403417b5625..d92d8e110b4d 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -63,8 +63,18 @@ static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting,\n \tfilter_refs(&array, filter, FILTER_REFS_TAGS);\n \tref_array_sort(sorting, &array);\n \n-\tfor (i = 0; i < array.nr; i++)\n-\t\tshow_ref_array_item(array.items[i], format);\n+\tfor (i = 0; i < array.nr; i++) {\n+\t\tstruct strbuf output = STRBUF_INIT;\n+\t\tstruct strbuf err = STRBUF_INIT;\n+\n+\t\tif (format_ref_array_item(array.items[i], format, &output, &err))\n+\t\t\tdie(\"%s\", err.buf);\n+\t\tfwrite(output.buf, 1, output.len, stdout);\n+\t\tputchar('\\n');\n+\n+\t\tstrbuf_release(&err);\n+\t\tstrbuf_release(&output);\n+\t}\n \tref_array_clear(&array);\n \tfree(to_free);\n \ndiff --git a/ref-filter.c b/ref-filter.c\nindex f0bd32f71416..df67047fd615 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -2435,27 +2435,22 @@ int format_ref_array_item(struct ref_array_item *info,\n \treturn 0;\n }\n \n-void show_ref_array_item(struct ref_array_item *info,\n-\t\t\t const struct ref_format *format)\n-{\n-\tstruct strbuf final_buf = STRBUF_INIT;\n-\tstruct strbuf error_buf = STRBUF_INIT;\n-\n-\tif (format_ref_array_item(info, format, &final_buf, &error_buf))\n-\t\tdie(\"%s\", error_buf.buf);\n-\tfwrite(final_buf.buf, 1, final_buf.len, stdout);\n-\tstrbuf_release(&error_buf);\n-\tstrbuf_release(&final_buf);\n-\tputchar('\\n');\n-}\n-\n void pretty_print_ref(const char *name, const struct object_id *oid,\n \t\t const struct ref_format *format)\n {\n \tstruct ref_array_item *ref_item;\n+\tstruct strbuf output = STRBUF_INIT;\n+\tstruct strbuf err = STRBUF_INIT;\n+\n \tref_item = new_ref_array_item(name, oid);\n \tref_item->kind = ref_kind_from_refname(name);\n-\tshow_ref_array_item(ref_item, format);\n+\tif (format_ref_array_item(ref_item, format, &output, &err))\n+\t\tdie(\"%s\", err.buf);\n+\tfwrite(output.buf, 1, output.len, stdout);\n+\tputchar('\\n');\n+\n+\tstrbuf_release(&err);\n+\tstrbuf_release(&output);\n \tfree_array_item(ref_item);\n }\n \ndiff --git a/ref-filter.h b/ref-filter.h\nindex 19ea4c413409..baf72a718965 100644\n--- a/ref-filter.h\n+++ b/ref-filter.h\n@@ -119,8 +119,6 @@ int format_ref_array_item(struct ref_array_item *info,\n \t\t\t const struct ref_format *format,\n \t\t\t struct strbuf *final_buf,\n \t\t\t struct strbuf *error_buf);\n-/* Print the ref using the given format and quote_style */\n-void show_ref_array_item(struct ref_array_item *info, const struct ref_format *format);\n /* Parse a single sort specifier and add it to the list */\n void parse_ref_sorting(struct ref_sorting **sorting_tail, const char *atom);\n /* Callback function for parsing the sort option */\n-- \ngitgitgadget\n\n"},{"id":"422266","messageId":"1c7a69ba072ac740273ef06972122f74cf3fa684.1618831726.git.gitgitgadget@gmail.com","threadId":"55517","inReplyTo":"pull.935.git.1618831726.gitgitgadget@gmail.com","subject":"[PATCH 2/2] [GSOC] ref-filter: reuse output buffer","fromName":"ZheNing Hu via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-04-19T11:28:45Z","receivedAt":"2021-04-19T11:28:54Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"From: ZheNing Hu \n\nWhen we use `git for-each-ref`, every ref will allocate\nits own output strbuf. But we can reuse the final strbuf\nfor each step ref's output.\n\nThe performance for `git for-each-ref` on the Git repository\nitself with performance testing tool `hyperfine` changes from\n23.7 ms ± 0.9 ms to 22.2 ms ± 1.0 ms. Optimization is relatively\nminor.\n\nAt the same time, we apply this optimization to `git tag -l`\nand `git branch -l`.\n\nThis approach is similar to the one used by 79ed0a5\n(cat-file: use a single strbuf for all output, 2018-08-14)\nto speed up the cat-file builtin.\n\nHelped-by: Jeff King \nHelped-by: René Scharfe \nSigned-off-by: ZheNing Hu \n---\n builtin/branch.c | 9 +++++----\n builtin/for-each-ref.c | 12 ++++++------\n builtin/tag.c | 12 ++++++------\n 3 files changed, 17 insertions(+), 16 deletions(-)\n\ndiff --git a/builtin/branch.c b/builtin/branch.c\nindex bcc00bcf182d..00081de1aed8 100644\n--- a/builtin/branch.c\n+++ b/builtin/branch.c\n@@ -411,6 +411,8 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin\n {\n \tint i;\n \tstruct ref_array array;\n+\tstruct strbuf out = STRBUF_INIT;\n+\tstruct strbuf err = STRBUF_INIT;\n \tint maxwidth = 0;\n \tconst char *remote_prefix = \"\";\n \tchar *to_free = NULL;\n@@ -440,8 +442,7 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin\n \tref_array_sort(sorting, &array);\n \n \tfor (i = 0; i < array.nr; i++) {\n-\t\tstruct strbuf out = STRBUF_INIT;\n-\t\tstruct strbuf err = STRBUF_INIT;\n+\t\tstrbuf_reset(&out);\n \t\tif (format_ref_array_item(array.items[i], format, &out, &err))\n \t\t\tdie(\"%s\", err.buf);\n \t\tif (column_active(colopts)) {\n@@ -452,10 +453,10 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin\n \t\t\tfwrite(out.buf, 1, out.len, stdout);\n \t\t\tputchar('\\n');\n \t\t}\n-\t\tstrbuf_release(&err);\n-\t\tstrbuf_release(&out);\n \t}\n \n+\tstrbuf_release(&err);\n+\tstrbuf_release(&out);\n \tref_array_clear(&array);\n \tfree(to_free);\n }\ndiff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c\nindex 8520008604e3..bf24c595c526 100644\n--- a/builtin/for-each-ref.c\n+++ b/builtin/for-each-ref.c\n@@ -22,6 +22,8 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)\n \tstruct ref_array array;\n \tstruct ref_filter filter;\n \tstruct ref_format format = REF_FORMAT_INIT;\n+\tstruct strbuf output = STRBUF_INIT;\n+\tstruct strbuf err = STRBUF_INIT;\n \n \tstruct option opts[] = {\n \t\tOPT_BIT('s', \"shell\", &format.quote_style,\n@@ -81,17 +83,15 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)\n \tif (!maxcount || array.nr < maxcount)\n \t\tmaxcount = array.nr;\n \tfor (i = 0; i < maxcount; i++) {\n-\t\tstruct strbuf output = STRBUF_INIT;\n-\t\tstruct strbuf err = STRBUF_INIT;\n-\n+\t\tstrbuf_reset(&output);\n \t\tif (format_ref_array_item(array.items[i], &format, &output, &err))\n \t\t\tdie(\"%s\", err.buf);\n \t\tfwrite(output.buf, 1, output.len, stdout);\n \t\tputchar('\\n');\n-\n-\t\tstrbuf_release(&err);\n-\t\tstrbuf_release(&output);\n \t}\n+\n+\tstrbuf_release(&err);\n+\tstrbuf_release(&output);\n \tref_array_clear(&array);\n \treturn 0;\n }\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex d92d8e110b4d..592af1d154ea 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -39,6 +39,8 @@ static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting,\n \t\t struct ref_format *format)\n {\n \tstruct ref_array array;\n+\tstruct strbuf output = STRBUF_INIT;\n+\tstruct strbuf err = STRBUF_INIT;\n \tchar *to_free = NULL;\n \tint i;\n \n@@ -64,17 +66,15 @@ static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting,\n \tref_array_sort(sorting, &array);\n \n \tfor (i = 0; i < array.nr; i++) {\n-\t\tstruct strbuf output = STRBUF_INIT;\n-\t\tstruct strbuf err = STRBUF_INIT;\n-\n+\t\tstrbuf_reset(&output);\n \t\tif (format_ref_array_item(array.items[i], format, &output, &err))\n \t\t\tdie(\"%s\", err.buf);\n \t\tfwrite(output.buf, 1, output.len, stdout);\n \t\tputchar('\\n');\n-\n-\t\tstrbuf_release(&err);\n-\t\tstrbuf_release(&output);\n \t}\n+\n+\tstrbuf_release(&err);\n+\tstrbuf_release(&output);\n \tref_array_clear(&array);\n \tfree(to_free);\n \n-- \ngitgitgadget\n"},{"id":"422338","messageId":"xmqqczuq9er5.fsf@gitster.g","threadId":"55517","inReplyTo":"ec98f2177d905cc9b3c61e15ddb756243536c125.1618831726.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/2] [GSOC] ref-filter: get rid of show_ref_array_item","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-04-19T20:54:38Z","receivedAt":"2021-04-19T20:55:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"ZheNing Hu via GitGitGadget\" writes:\n\n> From: ZheNing Hu \n>\n> Inlining the exported function `show_ref_array_item()`\n> which is not providing the right level of abstraction,\n> simplifies the API and can unlock improvements at the\n> former call sites.\n\nI'll insert a comma right at the end of the first line while\nqueuing.\n\nThanks.\n"},{"id":"422339","messageId":"xmqq7dky9ear.fsf@gitster.g","threadId":"55517","inReplyTo":"1c7a69ba072ac740273ef06972122f74cf3fa684.1618831726.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/2] [GSOC] ref-filter: reuse output buffer","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-04-19T21:04:28Z","receivedAt":"2021-04-19T21:04:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"ZheNing Hu via GitGitGadget\" writes:\n\n> diff --git a/builtin/branch.c b/builtin/branch.c\n> index bcc00bcf182d..00081de1aed8 100644\n> --- a/builtin/branch.c\n> +++ b/builtin/branch.c\n> @@ -411,6 +411,8 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin\n> {\n> \tint i;\n> \tstruct ref_array array;\n> +\tstruct strbuf out = STRBUF_INIT;\n> +\tstruct strbuf err = STRBUF_INIT;\n> \tint maxwidth = 0;\n> \tconst char *remote_prefix = \"\";\n> \tchar *to_free = NULL;\n> @@ -440,8 +442,7 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin\n> \tref_array_sort(sorting, &array);\n> \n> \tfor (i = 0; i < array.nr; i++) {\n> -\t\tstruct strbuf out = STRBUF_INIT;\n> -\t\tstruct strbuf err = STRBUF_INIT;\n> +\t\tstrbuf_reset(&out);\n> \t\tif (format_ref_array_item(array.items[i], format, &out, &err))\n> \t\t\tdie(\"%s\", err.buf);\n\nThis change relies on the fact that format_ref_array_item() will\nnever touch error when it returns 0 (success); otherwise, we'd end\nup accumulating err from multiple calls to it in the loop until it\nreturns non-zero (failure), at which point we emit a single \"fatal:\"\nprefix to show multiple error messages. Which leans me ...\n\n> @@ -452,10 +453,10 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin\n> \t\t\tfwrite(out.buf, 1, out.len, stdout);\n> \t\t\tputchar('\\n');\n> \t\t}\n> -\t\tstrbuf_release(&err);\n> -\t\tstrbuf_release(&out);\n> \t}\n> \n> +\tstrbuf_release(&err);\n> +\tstrbuf_release(&out);\n\n... to suspect that the _release() of err will always be a no-op.\n\nIt may be easier to follow if err is _reset() always where out is\n_reset(), from code cleanliness's perspective. Then nobody has to\nwonder why we do not reset err inside loop even though we release\nat the end.\n\nIt also is OK to document more clearly that we assume that the loop\nwill not exit without calling die() when err is not empty. If we\ntake that route, we may want to drop _release(&err) at the end.\n\nI do not know which of the two is better, but the code presented\nwhich is halfway between these two does not quite look easy to\nreason about.\n\nThanks.\n\n"},{"id":"422367","messageId":"CAOLTT8SDv6bFOdqu6f_=QWPrQtbyFp_XYFkdRmONg4tJ7C=fAA@mail.gmail.com","threadId":"55517","inReplyTo":"xmqq7dky9ear.fsf@gitster.g","subject":"Re: [PATCH 2/2] [GSOC] ref-filter: reuse output buffer","fromName":"ZheNing Hu","fromEmail":"adlternative@gmail.com","sentAt":"2021-04-20T06:05:07Z","receivedAt":"2021-04-20T06:05:22Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"Junio C Hamano 于2021年4月20日周二 上午5:04写道：\n>\n> \"ZheNing Hu via GitGitGadget\" writes:\n>\n> > diff --git a/builtin/branch.c b/builtin/branch.c\n> > index bcc00bcf182d..00081de1aed8 100644\n> > --- a/builtin/branch.c\n> > +++ b/builtin/branch.c\n> > @@ -411,6 +411,8 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin\n> > {\n> > int i;\n> > struct ref_array array;\n> > + struct strbuf out = STRBUF_INIT;\n> > + struct strbuf err = STRBUF_INIT;\n> > int maxwidth = 0;\n> > const char *remote_prefix = \"\";\n> > char *to_free = NULL;\n> > @@ -440,8 +442,7 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin\n> > ref_array_sort(sorting, &array);\n> >\n> > for (i = 0; i < array.nr; i++) {\n> > - struct strbuf out = STRBUF_INIT;\n> > - struct strbuf err = STRBUF_INIT;\n> > + strbuf_reset(&out);\n> > if (format_ref_array_item(array.items[i], format, &out, &err))\n> > die(\"%s\", err.buf);\n>\n> This change relies on the fact that format_ref_array_item() will\n> never touch error when it returns 0 (success); otherwise, we'd end\n> up accumulating err from multiple calls to it in the loop until it\n> returns non-zero (failure), at which point we emit a single \"fatal:\"\n> prefix to show multiple error messages. Which leans me ...\n>\n> > @@ -452,10 +453,10 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin\n> > fwrite(out.buf, 1, out.len, stdout);\n> > putchar('\\n');\n> > }\n> > - strbuf_release(&err);\n> > - strbuf_release(&out);\n> > }\n> >\n> > + strbuf_release(&err);\n> > + strbuf_release(&out);\n>\n> ... to suspect that the _release() of err will always be a no-op.\n>\n\nYes, it's a no-op to _release(&err) In the present situation.\n\n> It may be easier to follow if err is _reset() always where out is\n> _reset(), from code cleanliness's perspective. Then nobody has to\n> wonder why we do not reset err inside loop even though we release\n> at the end.\n>\n> It also is OK to document more clearly that we assume that the loop\n> will not exit without calling die() when err is not empty. If we\n> take that route, we may want to drop _release(&err) at the end.\n>\n> I do not know which of the two is better, but the code presented\n> which is halfway between these two does not quite look easy to\n> reason about.\n>\n\nRené Scharfe mention that it make leaks checking harder if we without\nreleasing this err. So on balance, adding err's _reset() in the loop seems\nlike a viable option. The change in performance will also be minimal too.\n\nEven though we're using _release() in the loop in v1, and then Peff think that\nwe don't need to _release() err, but code cleanness wasn't a concern\nat the time.\n\nSo I'll add _reset() to the loop in the next iteration.\n\n> Thanks.\n>\n\nThanks.\n--\nZheNing Hu\n"},{"id":"422453","messageId":"pull.935.v2.git.1618937532.gitgitgadget@gmail.com","threadId":"55517","inReplyTo":"pull.935.git.1618831726.gitgitgadget@gmail.com","subject":"[PATCH v2 0/2] [GSOC] ref-filter: reuse output buffer","fromName":"ZheNing Hu via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-04-20T16:52:09Z","receivedAt":"2021-04-20T16:52:21Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":" * Firstly, inlining show_ref_array_item().\n * Secondly, git for-each-ref reuse final buf for all refs output, the\n performance is slightly improved, this optimization is also applied to\n git tag -l and git branch -l.\n\nChanges made in v2: reset err buffer in loop for code cleanness.\n\nThanks.\n\nZheNing Hu (2):\n [GSOC] ref-filter: get rid of show_ref_array_item\n [GSOC] ref-filter: reuse output buffer\n\n builtin/branch.c | 10 ++++++----\n builtin/for-each-ref.c | 15 +++++++++++++--\n builtin/tag.c | 15 +++++++++++++--\n ref-filter.c | 25 ++++++++++---------------\n ref-filter.h | 2 --\n 5 files changed, 42 insertions(+), 25 deletions(-)\n\n\nbase-commit: 2e36527f23b7f6ae15e6f21ac3b08bf3fed6ee48\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-935%2Fadlternative%2Fref-filter-opt-reuse-buf-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-935/adlternative/ref-filter-opt-reuse-buf-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/935\n\nRange-diff vs v1:\n\n 1: ec98f2177d90 ! 1: 7e378eef4b34 [GSOC] ref-filter: get rid of show_ref_array_item\n @@ Metadata\n ## Commit message ##\n [GSOC] ref-filter: get rid of show_ref_array_item\n \n - Inlining the exported function `show_ref_array_item()`\n + Inlining the exported function `show_ref_array_item()`,\n which is not providing the right level of abstraction,\n simplifies the API and can unlock improvements at the\n former call sites.\n 2: 1c7a69ba072a ! 2: a17949b9f809 [GSOC] ref-filter: reuse output buffer\n @@ Commit message\n [GSOC] ref-filter: reuse output buffer\n \n When we use `git for-each-ref`, every ref will allocate\n - its own output strbuf. But we can reuse the final strbuf\n - for each step ref's output.\n + its own output strbuf and error strbuf. But we can reuse\n + the final strbuf for each step ref's output. The error\n + buffer will also be reused, despite the fact that the git\n + will exit when `format_ref_array_item()` return a non-zero\n + value and output the contents of the error buffer.\n \n The performance for `git for-each-ref` on the Git repository\n itself with performance testing tool `hyperfine` changes from\n @@ Commit message\n (cat-file: use a single strbuf for all output, 2018-08-14)\n to speed up the cat-file builtin.\n \n + Helped-by: Junio C Hamano \n Helped-by: Jeff King \n Helped-by: René Scharfe \n Signed-off-by: ZheNing Hu \n @@ builtin/branch.c: static void print_ref_list(struct ref_filter *filter, struct r\n \tfor (i = 0; i < array.nr; i++) {\n -\t\tstruct strbuf out = STRBUF_INIT;\n -\t\tstruct strbuf err = STRBUF_INIT;\n ++\t\tstrbuf_reset(&err);\n +\t\tstrbuf_reset(&out);\n \t\tif (format_ref_array_item(array.items[i], format, &out, &err))\n \t\t\tdie(\"%s\", err.buf);\n @@ builtin/for-each-ref.c: int cmd_for_each_ref(int argc, const char **argv, const\n -\t\tstruct strbuf output = STRBUF_INIT;\n -\t\tstruct strbuf err = STRBUF_INIT;\n -\n ++\t\tstrbuf_reset(&err);\n +\t\tstrbuf_reset(&output);\n \t\tif (format_ref_array_item(array.items[i], &format, &output, &err))\n \t\t\tdie(\"%s\", err.buf);\n @@ builtin/tag.c: static int list_tags(struct ref_filter *filter, struct ref_sortin\n -\t\tstruct strbuf err = STRBUF_INIT;\n -\n +\t\tstrbuf_reset(&output);\n ++\t\tstrbuf_reset(&err);\n \t\tif (format_ref_array_item(array.items[i], format, &output, &err))\n \t\t\tdie(\"%s\", err.buf);\n \t\tfwrite(output.buf, 1, output.len, stdout);\n\n-- \ngitgitgadget\n"},{"id":"422454","messageId":"a17949b9f80992a3f7ebfbf9fa602730abaf8bb1.1618937532.git.gitgitgadget@gmail.com","threadId":"55517","inReplyTo":"pull.935.v2.git.1618937532.gitgitgadget@gmail.com","subject":"[PATCH v2 2/2] [GSOC] ref-filter: reuse output buffer","fromName":"ZheNing Hu via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-04-20T16:52:11Z","receivedAt":"2021-04-20T16:52:23Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"From: ZheNing Hu \n\nWhen we use `git for-each-ref`, every ref will allocate\nits own output strbuf and error strbuf. But we can reuse\nthe final strbuf for each step ref's output. The error\nbuffer will also be reused, despite the fact that the git\nwill exit when `format_ref_array_item()` return a non-zero\nvalue and output the contents of the error buffer.\n\nThe performance for `git for-each-ref` on the Git repository\nitself with performance testing tool `hyperfine` changes from\n23.7 ms ± 0.9 ms to 22.2 ms ± 1.0 ms. Optimization is relatively\nminor.\n\nAt the same time, we apply this optimization to `git tag -l`\nand `git branch -l`.\n\nThis approach is similar to the one used by 79ed0a5\n(cat-file: use a single strbuf for all output, 2018-08-14)\nto speed up the cat-file builtin.\n\nHelped-by: Junio C Hamano \nHelped-by: Jeff King \nHelped-by: René Scharfe \nSigned-off-by: ZheNing Hu \n---\n builtin/branch.c | 10 ++++++----\n builtin/for-each-ref.c | 13 +++++++------\n builtin/tag.c | 13 +++++++------\n 3 files changed, 20 insertions(+), 16 deletions(-)\n\ndiff --git a/builtin/branch.c b/builtin/branch.c\nindex bcc00bcf182d..b23b1d1752af 100644\n--- a/builtin/branch.c\n+++ b/builtin/branch.c\n@@ -411,6 +411,8 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin\n {\n \tint i;\n \tstruct ref_array array;\n+\tstruct strbuf out = STRBUF_INIT;\n+\tstruct strbuf err = STRBUF_INIT;\n \tint maxwidth = 0;\n \tconst char *remote_prefix = \"\";\n \tchar *to_free = NULL;\n@@ -440,8 +442,8 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin\n \tref_array_sort(sorting, &array);\n \n \tfor (i = 0; i < array.nr; i++) {\n-\t\tstruct strbuf out = STRBUF_INIT;\n-\t\tstruct strbuf err = STRBUF_INIT;\n+\t\tstrbuf_reset(&err);\n+\t\tstrbuf_reset(&out);\n \t\tif (format_ref_array_item(array.items[i], format, &out, &err))\n \t\t\tdie(\"%s\", err.buf);\n \t\tif (column_active(colopts)) {\n@@ -452,10 +454,10 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin\n \t\t\tfwrite(out.buf, 1, out.len, stdout);\n \t\t\tputchar('\\n');\n \t\t}\n-\t\tstrbuf_release(&err);\n-\t\tstrbuf_release(&out);\n \t}\n \n+\tstrbuf_release(&err);\n+\tstrbuf_release(&out);\n \tref_array_clear(&array);\n \tfree(to_free);\n }\ndiff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c\nindex 8520008604e3..b529228c6239 100644\n--- a/builtin/for-each-ref.c\n+++ b/builtin/for-each-ref.c\n@@ -22,6 +22,8 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)\n \tstruct ref_array array;\n \tstruct ref_filter filter;\n \tstruct ref_format format = REF_FORMAT_INIT;\n+\tstruct strbuf output = STRBUF_INIT;\n+\tstruct strbuf err = STRBUF_INIT;\n \n \tstruct option opts[] = {\n \t\tOPT_BIT('s', \"shell\", &format.quote_style,\n@@ -81,17 +83,16 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)\n \tif (!maxcount || array.nr < maxcount)\n \t\tmaxcount = array.nr;\n \tfor (i = 0; i < maxcount; i++) {\n-\t\tstruct strbuf output = STRBUF_INIT;\n-\t\tstruct strbuf err = STRBUF_INIT;\n-\n+\t\tstrbuf_reset(&err);\n+\t\tstrbuf_reset(&output);\n \t\tif (format_ref_array_item(array.items[i], &format, &output, &err))\n \t\t\tdie(\"%s\", err.buf);\n \t\tfwrite(output.buf, 1, output.len, stdout);\n \t\tputchar('\\n');\n-\n-\t\tstrbuf_release(&err);\n-\t\tstrbuf_release(&output);\n \t}\n+\n+\tstrbuf_release(&err);\n+\tstrbuf_release(&output);\n \tref_array_clear(&array);\n \treturn 0;\n }\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex d92d8e110b4d..82fcfc098242 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -39,6 +39,8 @@ static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting,\n \t\t struct ref_format *format)\n {\n \tstruct ref_array array;\n+\tstruct strbuf output = STRBUF_INIT;\n+\tstruct strbuf err = STRBUF_INIT;\n \tchar *to_free = NULL;\n \tint i;\n \n@@ -64,17 +66,16 @@ static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting,\n \tref_array_sort(sorting, &array);\n \n \tfor (i = 0; i < array.nr; i++) {\n-\t\tstruct strbuf output = STRBUF_INIT;\n-\t\tstruct strbuf err = STRBUF_INIT;\n-\n+\t\tstrbuf_reset(&output);\n+\t\tstrbuf_reset(&err);\n \t\tif (format_ref_array_item(array.items[i], format, &output, &err))\n \t\t\tdie(\"%s\", err.buf);\n \t\tfwrite(output.buf, 1, output.len, stdout);\n \t\tputchar('\\n');\n-\n-\t\tstrbuf_release(&err);\n-\t\tstrbuf_release(&output);\n \t}\n+\n+\tstrbuf_release(&err);\n+\tstrbuf_release(&output);\n \tref_array_clear(&array);\n \tfree(to_free);\n \n-- \ngitgitgadget\n"},{"id":"422455","messageId":"7e378eef4b3405777eef9c0e9fe4e9e4ec6a1df9.1618937532.git.gitgitgadget@gmail.com","threadId":"55517","inReplyTo":"pull.935.v2.git.1618937532.gitgitgadget@gmail.com","subject":"[PATCH v2 1/2] [GSOC] ref-filter: get rid of show_ref_array_item","fromName":"ZheNing Hu via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-04-20T16:52:10Z","receivedAt":"2021-04-20T16:52:24Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"From: ZheNing Hu \n\nInlining the exported function `show_ref_array_item()`,\nwhich is not providing the right level of abstraction,\nsimplifies the API and can unlock improvements at the\nformer call sites.\n\nHelped-by: René Scharfe \nSigned-off-by: ZheNing Hu \n---\n builtin/for-each-ref.c | 14 ++++++++++++--\n builtin/tag.c | 14 ++++++++++++--\n ref-filter.c | 25 ++++++++++---------------\n ref-filter.h | 2 --\n 4 files changed, 34 insertions(+), 21 deletions(-)\n\ndiff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c\nindex cb9c81a04606..8520008604e3 100644\n--- a/builtin/for-each-ref.c\n+++ b/builtin/for-each-ref.c\n@@ -80,8 +80,18 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)\n \n \tif (!maxcount || array.nr < maxcount)\n \t\tmaxcount = array.nr;\n-\tfor (i = 0; i < maxcount; i++)\n-\t\tshow_ref_array_item(array.items[i], &format);\n+\tfor (i = 0; i < maxcount; i++) {\n+\t\tstruct strbuf output = STRBUF_INIT;\n+\t\tstruct strbuf err = STRBUF_INIT;\n+\n+\t\tif (format_ref_array_item(array.items[i], &format, &output, &err))\n+\t\t\tdie(\"%s\", err.buf);\n+\t\tfwrite(output.buf, 1, output.len, stdout);\n+\t\tputchar('\\n');\n+\n+\t\tstrbuf_release(&err);\n+\t\tstrbuf_release(&output);\n+\t}\n \tref_array_clear(&array);\n \treturn 0;\n }\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex d403417b5625..d92d8e110b4d 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -63,8 +63,18 @@ static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting,\n \tfilter_refs(&array, filter, FILTER_REFS_TAGS);\n \tref_array_sort(sorting, &array);\n \n-\tfor (i = 0; i < array.nr; i++)\n-\t\tshow_ref_array_item(array.items[i], format);\n+\tfor (i = 0; i < array.nr; i++) {\n+\t\tstruct strbuf output = STRBUF_INIT;\n+\t\tstruct strbuf err = STRBUF_INIT;\n+\n+\t\tif (format_ref_array_item(array.items[i], format, &output, &err))\n+\t\t\tdie(\"%s\", err.buf);\n+\t\tfwrite(output.buf, 1, output.len, stdout);\n+\t\tputchar('\\n');\n+\n+\t\tstrbuf_release(&err);\n+\t\tstrbuf_release(&output);\n+\t}\n \tref_array_clear(&array);\n \tfree(to_free);\n \ndiff --git a/ref-filter.c b/ref-filter.c\nindex f0bd32f71416..df67047fd615 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -2435,27 +2435,22 @@ int format_ref_array_item(struct ref_array_item *info,\n \treturn 0;\n }\n \n-void show_ref_array_item(struct ref_array_item *info,\n-\t\t\t const struct ref_format *format)\n-{\n-\tstruct strbuf final_buf = STRBUF_INIT;\n-\tstruct strbuf error_buf = STRBUF_INIT;\n-\n-\tif (format_ref_array_item(info, format, &final_buf, &error_buf))\n-\t\tdie(\"%s\", error_buf.buf);\n-\tfwrite(final_buf.buf, 1, final_buf.len, stdout);\n-\tstrbuf_release(&error_buf);\n-\tstrbuf_release(&final_buf);\n-\tputchar('\\n');\n-}\n-\n void pretty_print_ref(const char *name, const struct object_id *oid,\n \t\t const struct ref_format *format)\n {\n \tstruct ref_array_item *ref_item;\n+\tstruct strbuf output = STRBUF_INIT;\n+\tstruct strbuf err = STRBUF_INIT;\n+\n \tref_item = new_ref_array_item(name, oid);\n \tref_item->kind = ref_kind_from_refname(name);\n-\tshow_ref_array_item(ref_item, format);\n+\tif (format_ref_array_item(ref_item, format, &output, &err))\n+\t\tdie(\"%s\", err.buf);\n+\tfwrite(output.buf, 1, output.len, stdout);\n+\tputchar('\\n');\n+\n+\tstrbuf_release(&err);\n+\tstrbuf_release(&output);\n \tfree_array_item(ref_item);\n }\n \ndiff --git a/ref-filter.h b/ref-filter.h\nindex 19ea4c413409..baf72a718965 100644\n--- a/ref-filter.h\n+++ b/ref-filter.h\n@@ -119,8 +119,6 @@ int format_ref_array_item(struct ref_array_item *info,\n \t\t\t const struct ref_format *format,\n \t\t\t struct strbuf *final_buf,\n \t\t\t struct strbuf *error_buf);\n-/* Print the ref using the given format and quote_style */\n-void show_ref_array_item(struct ref_array_item *info, const struct ref_format *format);\n /* Parse a single sort specifier and add it to the list */\n void parse_ref_sorting(struct ref_sorting **sorting_tail, const char *atom);\n /* Callback function for parsing the sort option */\n-- \ngitgitgadget\n\n"}]}