{"thread":{"id":"56304","subject":"[PATCH 0/5] [GSOC] [RFC] ref-filter: performance optimization","startedAt":"2021-08-17T07:14:55Z","lastAt":"2021-08-17T07:15:00Z","messageCount":6,"participants":["ZheNing Hu via GitGitGadget"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"432904","messageId":"pull.1020.git.1629184489.gitgitgadget@gmail.com","threadId":"56304","inReplyTo":null,"subject":"[PATCH 0/5] [GSOC] [RFC] ref-filter: performance optimization","fromName":"ZheNing Hu via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-08-17T07:14:44Z","receivedAt":"2021-08-17T07:14:55Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"Last time I submitted a very long patch series:\nhttps://lore.kernel.org/git/pull.1016.git.1628842990.gitgitgadget@gmail.com/\nMy mentor Christian suggested to split the performance optimization part\nout, so this patch series used to optimize performance optimization in\nref-filter.\n\nChanges in this patch series:\n\n 1. Skip parse_object_buffer(), which can reduce object content parsing.\n 2. Use linked list to record parsing results, which can reduce second\n format string parsing.\n 3. Reuse final buffer, which can reduce memcpy();\n 4. Add a need_get_object_info flag instead of compare two object_info,\n which can reduce memcmp();\n 5. Use ALLOC_ARRAY() instead of CALLOC_ARRAY(), which can reduce memset();\n\nSince the relevant part of git cat-file --batch has been deleted, the\nexecution time of git for-each-ref is very short, I haven’t added\nperformance tests yet for git for-each-ref.\n\nZheNing Hu (5):\n [GSOC] ref-filter: skip parse_object_buffer in some cases\n [GSOC] ref-filter: remove second parsing in format_ref_array_item\n [GSOC] ref-filter: reuse final buffer\n [GSOC] ref-filter: reduce unnecessary object_info comparisons\n [GSOC]: ref-filter: instead CALLOC_ARRAY to ALLOC_ARRAY\n\n builtin/branch.c | 2 +\n builtin/for-each-ref.c | 3 +-\n builtin/tag.c | 2 +\n builtin/verify-tag.c | 2 +\n ref-filter.c | 168 ++++++++++++++++++++++++++++++++---------\n ref-filter.h | 17 ++++-\n 6 files changed, 154 insertions(+), 40 deletions(-)\n\n\nbase-commit: f000ecbed922c727382651490e75014f003c89ca\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1020%2Fadlternative%2Fref-filter-opt-perf-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1020/adlternative/ref-filter-opt-perf-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1020\n-- \ngitgitgadget\n"},{"id":"432905","messageId":"da3ba91c61a80c73d5c47abac3d807753e243288.1629184489.git.gitgitgadget@gmail.com","threadId":"56304","inReplyTo":"pull.1020.git.1629184489.gitgitgadget@gmail.com","subject":"[PATCH 1/5] [GSOC] ref-filter: skip parse_object_buffer in some cases","fromName":"ZheNing Hu via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-08-17T07:14:45Z","receivedAt":"2021-08-17T07:14:55Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"From: ZheNing Hu \n\nWhen we are using some atoms such as %(raw), %(objectname),\n%(objecttype), we don't need to parse the content of the object,\nso we can skip parse_object_buffer() for performance optimization.\n\nIt is worth noting that in these cases, we still need to call\nparse_object_buffer() for parsing:\n\n1. The atom type is one of %(tag), %(type), %(object), %(tree),\n%(numparent) or %(parent).\n2. The type of the object is tag and the atom need to be\ndereferenced e.g. %(*objecttype).\n\nMentored-by: Christian Couder \nMentored-by: Hariom Verma \nSigned-off-by: ZheNing Hu \n---\n ref-filter.c | 49 +++++++++++++++++++++++++++++++++++++------------\n ref-filter.h | 5 +++--\n 2 files changed, 40 insertions(+), 14 deletions(-)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 93ce2a6ef2e..65ba00633dc 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1009,6 +1009,20 @@ static int reject_atom(enum atom_type atom_type)\n \treturn atom_type == ATOM_REST;\n }\n \n+static int need_parse_buffer(enum atom_type atom_type) {\n+\tswitch (atom_type) {\n+\tcase ATOM_TAG:\n+\tcase ATOM_TYPE:\n+\tcase ATOM_OBJECT:\n+\tcase ATOM_TREE:\n+\tcase ATOM_NUMPARENT:\n+\tcase ATOM_PARENT:\n+\t\treturn 1;\n+\tdefault:\n+\t\treturn 0;\n+\t}\n+}\n+\n /*\n * Make sure the format string is well formed, and parse out\n * the used atoms.\n@@ -1029,6 +1043,8 @@ int verify_ref_format(struct ref_format *format)\n \t\tat = parse_ref_filter_atom(format, sp + 2, ep, &err);\n \t\tif (at < 0)\n \t\t\tdie(\"%s\", err.buf);\n+\t\tif (need_parse_buffer(used_atom[at].atom_type))\n+\t\t\tformat->can_skip_parse_buffer = 0;\n \t\tif (reject_atom(used_atom[at].atom_type))\n \t\t\tdie(_(\"this command reject atom %%(%.*s)\"), (int)(ep - sp - 2), sp + 2);\n \n@@ -1524,14 +1540,16 @@ static void grab_values(struct atom_value *val, int deref, struct object *obj, s\n {\n \tvoid *buf = data->content;\n \n-\tswitch (obj->type) {\n+\tswitch (data->type) {\n \tcase OBJ_TAG:\n-\t\tgrab_tag_values(val, deref, obj);\n+\t\tif (obj)\n+\t\t\tgrab_tag_values(val, deref, obj);\n \t\tgrab_sub_body_contents(val, deref, data);\n \t\tgrab_person(\"tagger\", val, deref, buf);\n \t\tbreak;\n \tcase OBJ_COMMIT:\n-\t\tgrab_commit_values(val, deref, obj);\n+\t\tif (obj)\n+\t\t\tgrab_commit_values(val, deref, obj);\n \t\tgrab_sub_body_contents(val, deref, data);\n \t\tgrab_person(\"author\", val, deref, buf);\n \t\tgrab_person(\"committer\", val, deref, buf);\n@@ -1757,14 +1775,21 @@ static int get_object(struct ref_array_item *ref, int deref, struct object **obj\n \t\tBUG(\"Object size is less than zero.\");\n \n \tif (oi->info.contentp) {\n-\t\t*obj = parse_object_buffer(the_repository, &oi->oid, oi->type, oi->size, oi->content, &eaten);\n-\t\tif (!*obj) {\n-\t\t\tif (!eaten)\n-\t\t\t\tfree(oi->content);\n-\t\t\treturn strbuf_addf_ret(err, -1, _(\"parse_object_buffer failed on %s for %s\"),\n-\t\t\t\t\t oid_to_hex(&oi->oid), ref->refname);\n+\t\tif (ref->can_skip_parse_buffer &&\n+\t\t ((!deref &&\n+\t\t (!need_tagged || oi->type != OBJ_TAG)) ||\n+\t\t deref)) {\n+\t\t\tgrab_values(ref->value, deref, NULL, oi);\n+\t\t} else {\n+\t\t\t*obj = parse_object_buffer(the_repository, &oi->oid, oi->type, oi->size, oi->content, &eaten);\n+\t\t\tif (!*obj) {\n+\t\t\t\tif (!eaten)\n+\t\t\t\t\tfree(oi->content);\n+\t\t\t\treturn strbuf_addf_ret(err, -1, _(\"parse_object_buffer failed on %s for %s\"),\n+\t\t\t\t\t\t oid_to_hex(&oi->oid), ref->refname);\n+\t\t\t}\n+\t\t\tgrab_values(ref->value, deref, *obj, oi);\n \t\t}\n-\t\tgrab_values(ref->value, deref, *obj, oi);\n \t}\n \n \tgrab_common_values(ref->value, deref, oi);\n@@ -1988,7 +2013,7 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)\n \t * If there is no atom that wants to know about tagged\n \t * object, we are done.\n \t */\n-\tif (!need_tagged || (obj->type != OBJ_TAG))\n+\tif (!need_tagged || (oi.type != OBJ_TAG))\n \t\treturn 0;\n \n \t/*\n@@ -2595,7 +2620,7 @@ int format_ref_array_item(struct ref_array_item *info,\n \n \tstate.quote_style = format->quote_style;\n \tpush_stack_element(&state.stack);\n-\n+\tinfo->can_skip_parse_buffer = format->can_skip_parse_buffer;\n \tfor (cp = format->format; *cp && (sp = find_next(cp)); cp = ep + 1) {\n \t\tstruct atom_value *atomv;\n \t\tint pos;\ndiff --git a/ref-filter.h b/ref-filter.h\nindex c15dee8d6b9..5bceae1dac9 100644\n--- a/ref-filter.h\n+++ b/ref-filter.h\n@@ -40,6 +40,7 @@ struct ref_array_item {\n \tstruct object_id objectname;\n \tconst char *rest;\n \tint flag;\n+\tint can_skip_parse_buffer;\n \tunsigned int kind;\n \tconst char *symref;\n \tstruct commit *commit;\n@@ -81,12 +82,12 @@ struct ref_format {\n \tint quote_style;\n \tint use_rest;\n \tint use_color;\n-\n+\tint can_skip_parse_buffer;\n \t/* Internal state to ref-filter */\n \tint need_color_reset_at_eol;\n };\n \n-#define REF_FORMAT_INIT { .use_color = -1 }\n+#define REF_FORMAT_INIT { .use_color = -1, .can_skip_parse_buffer = 1 }\n \n /* Macros for checking --merged and --no-merged options */\n #define _OPT_MERGED_NO_MERGED(option, filter, h) \\\n-- \ngitgitgadget\n\n"},{"id":"432906","messageId":"ce7f5329560a8ec29630e03e91b9f630d71a8391.1629184489.git.gitgitgadget@gmail.com","threadId":"56304","inReplyTo":"pull.1020.git.1629184489.gitgitgadget@gmail.com","subject":"[PATCH 2/5] [GSOC] ref-filter: remove second parsing in format_ref_array_item","fromName":"ZheNing Hu via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-08-17T07:14:46Z","receivedAt":"2021-08-17T07:14:57Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"From: ZheNing Hu \n\nWe parsed format string in verify_ref_format() and stored the parsed\natom in used_atom array. But in format_ref_array_item() we have\nanother round of parsing format string. This affects performance.\n\nIntroducing the struct parsed_atom_list which can save the current\natom's start and end address in format string and its index in\nused_atom. All parsed_atom_list entry are linked together in the\nform of linked list, and the head node of the linked list is stored\nin struct ref_format. Create clear_parsed_atom_list() which can used\nto clear the nodes on the linked list.\n\nThis can bring performance improvement.\n\nMentored-by: Christian Couder \nMentored-by: Hariom Verma \nSigned-off-by: ZheNing Hu \n---\n builtin/branch.c | 2 ++\n builtin/for-each-ref.c | 3 ++-\n builtin/tag.c | 2 ++\n builtin/verify-tag.c | 2 ++\n ref-filter.c | 45 +++++++++++++++++++++++++++++++++---------\n ref-filter.h | 11 +++++++++++\n 6 files changed, 55 insertions(+), 10 deletions(-)\n\ndiff --git a/builtin/branch.c b/builtin/branch.c\nindex b23b1d1752a..e361f8cc661 100644\n--- a/builtin/branch.c\n+++ b/builtin/branch.c\n@@ -459,6 +459,7 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin\n \tstrbuf_release(&err);\n \tstrbuf_release(&out);\n \tref_array_clear(&array);\n+\tclear_parsed_atom_list(&format->parsed_atom_head);\n \tfree(to_free);\n }\n \n@@ -678,6 +679,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n \tmemset(&filter, 0, sizeof(filter));\n \tfilter.kind = FILTER_REFS_BRANCHES;\n \tfilter.abbrev = -1;\n+\tINIT_LIST_HEAD(&format.parsed_atom_head);\n \n \tif (argc == 2 && !strcmp(argv[1], \"-h\"))\n \t\tusage_with_options(builtin_branch_usage, options);\ndiff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c\nindex 89cb6307d46..6e22d80d5b5 100644\n--- a/builtin/for-each-ref.c\n+++ b/builtin/for-each-ref.c\n@@ -53,8 +53,8 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)\n \n \tmemset(&array, 0, sizeof(array));\n \tmemset(&filter, 0, sizeof(filter));\n-\n \tformat.format = \"%(objectname) %(objecttype)\\t%(refname)\";\n+\tINIT_LIST_HEAD(&format.parsed_atom_head);\n \n \tgit_config(git_default_config, NULL);\n \n@@ -96,6 +96,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)\n \tref_array_clear(&array);\n \tfree_commit_list(filter.with_commit);\n \tfree_commit_list(filter.no_commit);\n+\tclear_parsed_atom_list(&format.parsed_atom_head);\n \tUNLEAK(sorting);\n \treturn 0;\n }\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex 452558ec957..549339cbbe4 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -78,6 +78,7 @@ static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting,\n \tstrbuf_release(&output);\n \tref_array_clear(&array);\n \tfree(to_free);\n+\tclear_parsed_atom_list(&format->parsed_atom_head);\n \n \treturn 0;\n }\n@@ -493,6 +494,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \tmemset(&filter, 0, sizeof(filter));\n \tfilter.lines = -1;\n \topt.sign = -1;\n+\tINIT_LIST_HEAD(&format.parsed_atom_head);\n \n \targc = parse_options(argc, argv, prefix, options, git_tag_usage, 0);\n \ndiff --git a/builtin/verify-tag.c b/builtin/verify-tag.c\nindex f45136a06ba..8b0a2b2587c 100644\n--- a/builtin/verify-tag.c\n+++ b/builtin/verify-tag.c\n@@ -39,6 +39,7 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix)\n \t\tOPT_END()\n \t};\n \n+\tINIT_LIST_HEAD(&format.parsed_atom_head);\n \tgit_config(git_verify_tag_config, NULL);\n \n \targc = parse_options(argc, argv, prefix, verify_tag_options,\n@@ -73,5 +74,6 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix)\n \t\tif (format.format)\n \t\t\tpretty_print_ref(name, &oid, &format);\n \t}\n+\tclear_parsed_atom_list(&format.parsed_atom_head);\n \treturn had_error;\n }\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 65ba00633dc..76a31fb79b1 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1035,6 +1035,7 @@ int verify_ref_format(struct ref_format *format)\n \tfor (cp = format->format; *cp && (sp = find_next(cp)); ) {\n \t\tstruct strbuf err = STRBUF_INIT;\n \t\tconst char *color, *ep = strchr(sp, ')');\n+\t\tstruct parsed_atom_list *e;\n \t\tint at;\n \n \t\tif (!ep)\n@@ -1043,6 +1044,12 @@ int verify_ref_format(struct ref_format *format)\n \t\tat = parse_ref_filter_atom(format, sp + 2, ep, &err);\n \t\tif (at < 0)\n \t\t\tdie(\"%s\", err.buf);\n+\t\te = xmalloc(sizeof(*e));\n+\t\te->beg = sp + 2;\n+\t\te->end = ep;\n+\t\te->at = at;\n+\t\tlist_add_tail(&e->list, &format->parsed_atom_head);\n+\n \t\tif (need_parse_buffer(used_atom[at].atom_type))\n \t\t\tformat->can_skip_parse_buffer = 0;\n \t\tif (reject_atom(used_atom[at].atom_type))\n@@ -2615,25 +2622,31 @@ int format_ref_array_item(struct ref_array_item *info,\n \t\t\t struct strbuf *final_buf,\n \t\t\t struct strbuf *error_buf)\n {\n-\tconst char *cp, *sp, *ep;\n+\tconst char *cp, *sp;\n+\tstruct list_head *item;\n \tstruct ref_formatting_state state = REF_FORMATTING_STATE_INIT;\n \n \tstate.quote_style = format->quote_style;\n \tpush_stack_element(&state.stack);\n \tinfo->can_skip_parse_buffer = format->can_skip_parse_buffer;\n-\tfor (cp = format->format; *cp && (sp = find_next(cp)); cp = ep + 1) {\n+\n+\tcp = format->format;\n+\n+\tlist_for_each(item, &format->parsed_atom_head) {\n \t\tstruct atom_value *atomv;\n-\t\tint pos;\n+\t\tstruct parsed_atom_list *e =\n+\t\t\tlist_entry(item, struct parsed_atom_list, list);\n \n-\t\tep = strchr(sp, ')');\n-\t\tif (cp < sp)\n-\t\t\tappend_literal(cp, sp, &state);\n-\t\tpos = parse_ref_filter_atom(format, sp + 2, ep, error_buf);\n-\t\tif (pos < 0 || get_ref_atom_value(info, pos, &atomv, error_buf) ||\n+\t\tif (cp < e->beg - 2)\n+\t\t\tappend_literal(cp, e->beg - 2, &state);\n+\t\tif (get_ref_atom_value(info, e->at, &atomv, error_buf) ||\n \t\t atomv->handler(atomv, &state, error_buf)) {\n \t\t\tpop_stack_element(&state.stack);\n \t\t\treturn -1;\n \t\t}\n+\t\tcp = e->end + 1;\n+\t\tif (!*cp)\n+\t\t\tbreak;\n \t}\n \tif (*cp) {\n \t\tsp = cp + strlen(cp);\n@@ -2681,10 +2694,12 @@ static int parse_sorting_atom(const char *atom)\n \t * This parses an atom using a dummy ref_format, since we don't\n \t * actually care about the formatting details.\n \t */\n+\tint res;\n \tstruct ref_format dummy = REF_FORMAT_INIT;\n \tconst char *end = atom + strlen(atom);\n \tstruct strbuf err = STRBUF_INIT;\n-\tint res = parse_ref_filter_atom(&dummy, atom, end, &err);\n+\n+\tres = parse_ref_filter_atom(&dummy, atom, end, &err);\n \tif (res < 0)\n \t\tdie(\"%s\", err.buf);\n \tstrbuf_release(&err);\n@@ -2757,3 +2772,15 @@ int parse_opt_merge_filter(const struct option *opt, const char *arg, int unset)\n \n \treturn 0;\n }\n+\n+void clear_parsed_atom_list(struct list_head *parsed_atom_head)\n+{\n+\tstruct list_head *pos, *tmp;\n+\tstruct parsed_atom_list *item;\n+\n+\tlist_for_each_safe(pos, tmp, parsed_atom_head) {\n+\t\titem = list_entry(pos, struct parsed_atom_list, list);\n+\t\tlist_del(pos);\n+\t\tfree(item);\n+\t}\n+}\ndiff --git a/ref-filter.h b/ref-filter.h\nindex 5bceae1dac9..df54836a643 100644\n--- a/ref-filter.h\n+++ b/ref-filter.h\n@@ -72,6 +72,13 @@ struct ref_filter {\n \t\tverbose;\n };\n \n+struct parsed_atom_list {\n+\tconst char *beg;\n+\tconst char *end;\n+\tint at;\n+\tstruct list_head list;\n+};\n+\n struct ref_format {\n \t/*\n \t * Set these to define the format; make sure you call\n@@ -85,6 +92,7 @@ struct ref_format {\n \tint can_skip_parse_buffer;\n \t/* Internal state to ref-filter */\n \tint need_color_reset_at_eol;\n+\tstruct list_head parsed_atom_head;\n };\n \n #define REF_FORMAT_INIT { .use_color = -1, .can_skip_parse_buffer = 1 }\n@@ -112,6 +120,9 @@ struct ref_format {\n int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int type);\n /* Clear all memory allocated to ref_array */\n void ref_array_clear(struct ref_array *array);\n+/* Clear the parsed_atom_list in ref_format*/\n+void clear_parsed_atom_list(struct list_head *parsed_atom_head);\n+\n /* Used to verify if the given format is correct and to parse out the used atoms */\n int verify_ref_format(struct ref_format *format);\n /* Sort the given ref_array as per the ref_sorting provided */\n-- \ngitgitgadget\n\n"},{"id":"432907","messageId":"3760ff032bb1dec3812881fd408f8d78ec125477.1629184489.git.gitgitgadget@gmail.com","threadId":"56304","inReplyTo":"pull.1020.git.1629184489.gitgitgadget@gmail.com","subject":"[PATCH 3/5] [GSOC] ref-filter: reuse final buffer","fromName":"ZheNing Hu via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-08-17T07:14:47Z","receivedAt":"2021-08-17T07:14:59Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"From: ZheNing Hu \n\nIn format_ref_array_item(), we add the object data\nto ref_formatting_state, and copy the data from\nref_formatting_state to final_buf at the end. There\nare huge copies of data.\n\nBecause final_buf will be cleared before every time\nwe call format_ref_array_item(). So we actually add\ncontent to an empty strbuf. We can add the object's\ndata directly to this final_buffer instead of adding\nobjects' data to state.stack->output first, then\ncopy to final_buf.\n\nAdd a can_reuse_final_buffer flag to struct ref_format\nand create can_reuse_final_buffer() to check if we are\nuse %(align), %(end), %(if), %(then), %(else). If not,\nwe can reuse the buf of finnal_buf.\n\nReuse the buffer address of final_buf in\nformat_ref_array_item(), we directly add the data to the\nfinal buffer, return the content to final_buf at the end.\n\nThis will bring performance improvements.\n\nMentored-by: Christian Couder \nMentored-by: Hariom Verma \nSigned-off-by: ZheNing Hu \n---\n ref-filter.c | 39 ++++++++++++++++++++++++++++++++++-----\n ref-filter.h | 3 ++-\n 2 files changed, 36 insertions(+), 6 deletions(-)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 76a31fb79b1..7106d4c1c4c 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1023,6 +1023,19 @@ static int need_parse_buffer(enum atom_type atom_type) {\n \t}\n }\n \n+static int can_reuse_final_buffer(enum atom_type atom_type) {\n+\tswitch (atom_type) {\n+\tcase ATOM_ALIGN:\n+\tcase ATOM_END:\n+\tcase ATOM_IF:\n+\tcase ATOM_THEN:\n+\tcase ATOM_ELSE:\n+\t\treturn 0;\n+\tdefault:\n+\t\treturn 1;\n+\t}\n+}\n+\n /*\n * Make sure the format string is well formed, and parse out\n * the used atoms.\n@@ -1054,6 +1067,7 @@ int verify_ref_format(struct ref_format *format)\n \t\t\tformat->can_skip_parse_buffer = 0;\n \t\tif (reject_atom(used_atom[at].atom_type))\n \t\t\tdie(_(\"this command reject atom %%(%.*s)\"), (int)(ep - sp - 2), sp + 2);\n+\t\tformat->can_reuse_final_buffer = can_reuse_final_buffer(used_atom[at].atom_type);\n \n \t\tif ((format->quote_style == QUOTE_PYTHON ||\n \t\t format->quote_style == QUOTE_SHELL ||\n@@ -2627,7 +2641,14 @@ int format_ref_array_item(struct ref_array_item *info,\n \tstruct ref_formatting_state state = REF_FORMATTING_STATE_INIT;\n \n \tstate.quote_style = format->quote_style;\n-\tpush_stack_element(&state.stack);\n+\tif (format->can_reuse_final_buffer) {\n+\t\tstruct ref_formatting_stack *s = xmalloc(sizeof(struct ref_formatting_stack));\n+\t\ts->output = *final_buf;\n+\t\ts->prev = state.stack;\n+\t\tstate.stack = s;\n+\t} else {\n+\t\tpush_stack_element(&state.stack);\n+\t}\n \tinfo->can_skip_parse_buffer = format->can_skip_parse_buffer;\n \n \tcp = format->format;\n@@ -2641,7 +2662,8 @@ int format_ref_array_item(struct ref_array_item *info,\n \t\t\tappend_literal(cp, e->beg - 2, &state);\n \t\tif (get_ref_atom_value(info, e->at, &atomv, error_buf) ||\n \t\t atomv->handler(atomv, &state, error_buf)) {\n-\t\t\tpop_stack_element(&state.stack);\n+\t\t\tif (!format->can_reuse_final_buffer)\n+\t\t\t\tpop_stack_element(&state.stack);\n \t\t\treturn -1;\n \t\t}\n \t\tcp = e->end + 1;\n@@ -2656,16 +2678,23 @@ int format_ref_array_item(struct ref_array_item *info,\n \t\tstruct atom_value resetv = ATOM_VALUE_INIT;\n \t\tresetv.s = GIT_COLOR_RESET;\n \t\tif (append_atom(&resetv, &state, error_buf)) {\n-\t\t\tpop_stack_element(&state.stack);\n+\t\t\tif (!format->can_reuse_final_buffer)\n+\t\t\t\tpop_stack_element(&state.stack);\n \t\t\treturn -1;\n \t\t}\n \t}\n \tif (state.stack->prev) {\n+\t\tassert(!format->can_reuse_final_buffer);\n \t\tpop_stack_element(&state.stack);\n \t\treturn strbuf_addf_ret(error_buf, -1, _(\"format: %%(end) atom missing\"));\n \t}\n-\tstrbuf_addbuf(final_buf, &state.stack->output);\n-\tpop_stack_element(&state.stack);\n+\tif(format->can_reuse_final_buffer) {\n+\t\t*final_buf = state.stack->output;\n+\t\tfree(state.stack);\n+\t} else {\n+\t\tstrbuf_addbuf(final_buf, &state.stack->output);\n+\t\tpop_stack_element(&state.stack);\n+\t}\n \treturn 0;\n }\n \ndiff --git a/ref-filter.h b/ref-filter.h\nindex df54836a643..a62a14a2e43 100644\n--- a/ref-filter.h\n+++ b/ref-filter.h\n@@ -92,10 +92,11 @@ struct ref_format {\n \tint can_skip_parse_buffer;\n \t/* Internal state to ref-filter */\n \tint need_color_reset_at_eol;\n+\tint can_reuse_final_buffer;\n \tstruct list_head parsed_atom_head;\n };\n \n-#define REF_FORMAT_INIT { .use_color = -1, .can_skip_parse_buffer = 1 }\n+#define REF_FORMAT_INIT { .use_color = -1, .can_skip_parse_buffer = 1, .can_reuse_final_buffer = 1 }\n \n /* Macros for checking --merged and --no-merged options */\n #define _OPT_MERGED_NO_MERGED(option, filter, h) \\\n-- \ngitgitgadget\n\n"},{"id":"432908","messageId":"3899fbe498f629d1af7dcea792b14eccdbb51af5.1629184489.git.gitgitgadget@gmail.com","threadId":"56304","inReplyTo":"pull.1020.git.1629184489.gitgitgadget@gmail.com","subject":"[PATCH 4/5] [GSOC] ref-filter: reduce unnecessary object_info comparisons","fromName":"ZheNing Hu via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-08-17T07:14:48Z","receivedAt":"2021-08-17T07:14:59Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"From: ZheNing Hu \n\nIn populate_value() we use an empty struct object_info\ncompare with oi and oi_deref, which determine if we can\nreturn early, this is actually very inefficient.\n\nSo add a need_get_object_info flag in global variables,\nwhich used to indicate whether we need call get_object()\nor we can return early.\n\nSo we only need to compare only one byte instead of comparing\nthe entire huge object_info struct.\n\nMentored-by: Christian Couder \nMentored-by: Hariom Verma \nSigned-off-by: ZheNing Hu \n---\n ref-filter.c | 34 +++++++++++++++++++++++-----------\n 1 file changed, 23 insertions(+), 11 deletions(-)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 7106d4c1c4c..35e221e7ec8 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -212,7 +212,7 @@ static struct used_atom {\n \t\tchar *head;\n \t} u;\n } *used_atom;\n-static int used_atom_cnt, need_tagged, need_symref;\n+static int used_atom_cnt, need_tagged, need_symref, need_get_object_info;\n \n /*\n * Expand string, append it to strbuf *sb, then return error code ret.\n@@ -318,10 +318,13 @@ static int objecttype_atom_parser(struct ref_format *format, struct used_atom *a\n {\n \tif (arg)\n \t\treturn strbuf_addf_ret(err, -1, _(\"%%(objecttype) does not take arguments\"));\n-\tif (*atom->name == '*')\n+\tif (*atom->name == '*') {\n \t\toi_deref.info.typep = &oi_deref.type;\n-\telse\n+\t\tneed_get_object_info = 1;\n+\t} else {\n \t\toi.info.typep = &oi.type;\n+\t\tneed_get_object_info = 1;\n+\t}\n \treturn 0;\n }\n \n@@ -330,16 +333,23 @@ static int objectsize_atom_parser(struct ref_format *format, struct used_atom *a\n {\n \tif (!arg) {\n \t\tatom->u.objectsize.option = O_SIZE;\n-\t\tif (*atom->name == '*')\n+\t\tif (*atom->name == '*') {\n \t\t\toi_deref.info.sizep = &oi_deref.size;\n-\t\telse\n+\t\t\tneed_get_object_info = 1;\n+\t\t} else {\n \t\t\toi.info.sizep = &oi.size;\n+\t\t\tneed_get_object_info = 1;\n+\t\t}\n \t} else if (!strcmp(arg, \"disk\")) {\n \t\tatom->u.objectsize.option = O_SIZE_DISK;\n-\t\tif (*atom->name == '*')\n+\t\tif (*atom->name == '*') {\n \t\t\toi_deref.info.disk_sizep = &oi_deref.disk_size;\n-\t\telse\n+\t\t\tneed_get_object_info = 1;\n+\t\t}\n+\t\telse {\n \t\t\toi.info.disk_sizep = &oi.disk_size;\n+\t\t\tneed_get_object_info = 1;\n+\t\t}\n \t} else\n \t\treturn strbuf_addf_ret(err, -1, _(\"unrecognized %%(objectsize) argument: %s\"), arg);\n \treturn 0;\n@@ -354,6 +364,7 @@ static int deltabase_atom_parser(struct ref_format *format, struct used_atom *at\n \t\toi_deref.info.delta_base_oid = &oi_deref.delta_base_oid;\n \telse\n \t\toi.info.delta_base_oid = &oi.delta_base_oid;\n+\tneed_get_object_info = 1;\n \treturn 0;\n }\n \n@@ -720,6 +731,7 @@ static int parse_ref_filter_atom(struct ref_format *format,\n \tused_atom[at].type = valid_atom[i].cmp_type;\n \tused_atom[at].source = valid_atom[i].source;\n \tif (used_atom[at].source == SOURCE_OBJ) {\n+\t\tneed_get_object_info = 1;\n \t\tif (*atom == '*')\n \t\t\toi_deref.info.contentp = &oi_deref.content;\n \t\telse\n@@ -1871,7 +1883,6 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)\n {\n \tstruct object *obj;\n \tint i;\n-\tstruct object_info empty = OBJECT_INFO_INIT;\n \n \tCALLOC_ARRAY(ref->value, used_atom_cnt);\n \n@@ -2019,10 +2030,11 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)\n \t\t\t\t\t oid_to_hex(&ref->objectname), ref->refname);\n \t}\n \n-\tif (need_tagged)\n+\tif (need_tagged) {\n \t\toi.info.contentp = &oi.content;\n-\tif (!memcmp(&oi.info, &empty, sizeof(empty)) &&\n-\t !memcmp(&oi_deref.info, &empty, sizeof(empty)))\n+\t\tneed_get_object_info = 1;\n+\t}\n+\tif (!need_get_object_info)\n \t\treturn 0;\n \n \n-- \ngitgitgadget\n\n"},{"id":"432909","messageId":"3476e548338623f954abd73192360bcba7df3d89.1629184489.git.gitgitgadget@gmail.com","threadId":"56304","inReplyTo":"pull.1020.git.1629184489.gitgitgadget@gmail.com","subject":"[PATCH 5/5] [GSOC]: ref-filter: instead CALLOC_ARRAY to ALLOC_ARRAY","fromName":"ZheNing Hu via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-08-17T07:14:49Z","receivedAt":"2021-08-17T07:15:00Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"From: ZheNing Hu \n\npopulate_value() uses CALLOC_ARRAY() to allocate dynamic memory for\nref->value. But after that, we immediately assign values to its\nmembers in the for loop. This shows that it is not necessary to\nuse CALLOC_ARRAY() for ref->value to clear the memory.\n\nTherefore, use ALLOC_ARRAY() instead of CALLOC_ARRAY() to reduce the\noverhead caused by clearing the memory. This can bring performance\noptimizations.\n\nMentored-by: Christian Couder \nMentored-by: Hariom Verma \nSigned-off-by: ZheNing Hu \n---\n ref-filter.c | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 35e221e7ec8..fe2df82067f 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1884,7 +1884,7 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)\n \tstruct object *obj;\n \tint i;\n \n-\tCALLOC_ARRAY(ref->value, used_atom_cnt);\n+\tALLOC_ARRAY(ref->value, used_atom_cnt);\n \n \tif (need_symref && (ref->flag & REF_ISSYMREF) && !ref->symref) {\n \t\tref->symref = resolve_refdup(ref->refname, RESOLVE_REF_READING,\n@@ -1903,6 +1903,7 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)\n \t\tconst char *refname;\n \t\tstruct branch *branch = NULL;\n \n+\t\tv->s = NULL;\n \t\tv->s_size = ATOM_SIZE_UNSPECIFIED;\n \t\tv->handler = append_atom;\n \t\tv->atom = atom;\n-- \ngitgitgadget\n"}]}