{"thread":{"id":"56307","subject":"[PATCH 0/8] [GSOC] [RFC] ref-filter: code logic optimization","startedAt":"2021-08-17T08:41:45Z","lastAt":"2021-08-17T08:41:55Z","messageCount":9,"participants":["ZheNing Hu via GitGitGadget"],"isPatch":true,"patchVersion":1,"patchTotal":8},"messages":[{"id":"432918","messageId":"pull.1021.git.1629189701.gitgitgadget@gmail.com","threadId":"56307","inReplyTo":null,"subject":"[PATCH 0/8] [GSOC] [RFC] ref-filter: code logic optimization","fromName":"ZheNing Hu via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-08-17T08:41:33Z","receivedAt":"2021-08-17T08:41:45Z","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 code logic in ref-filter.\n\nChanges in this patch series:\n\n 1. Remove grab_oid to reduce unnecessary string comparison.\n 2. Use atom_type in grab_person() and merge the two for loops.\n 3. Remove strlen() and introducing xstrvfmt_len() and xstrfmt_len() to\n    reduce strlen() overhead.\n 4. Introduction ref_filter_slopbuf[1] to reduce memory allocation.\n 5. Add deref to struct used_atom to increase the readability of the code.\n 6. Introduction symref_atom_parser() to increase the readability of the\n    code.\n 7. Use switch/case instread of if/else to increase the readability of the\n    code.\n\nZheNing Hu (8):\n  [GSOC] ref-filter: remove grab_oid() function\n  [GSOC] ref-filter: merge two for loop in grab_person\n  [GSOC] ref-filter: remove strlen from find_subpos\n  [GSOC] ref-filter: introducing xstrvfmt_len() and xstrfmt_len()\n  [GSOC] ref-filter: introduction ref_filter_slopbuf[1]\n  [GSOC] ref-filter: add deref member to struct used_atom\n  [GSOC] ref-filter: introduce symref_atom_parser()\n  [GSOC] ref-filter: use switch/case instead of if/else\n\n ref-filter.c | 432 ++++++++++++++++++++++++++++-----------------------\n 1 file changed, 237 insertions(+), 195 deletions(-)\n\n\nbase-commit: f000ecbed922c727382651490e75014f003c89ca\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1021%2Fadlternative%2Fref-filter-opt-code-logic-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1021/adlternative/ref-filter-opt-code-logic-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1021\n-- \ngitgitgadget\n"},{"id":"432919","messageId":"14c8a0bdb8051737bfade9c761a647daac7ea12a.1629189701.git.gitgitgadget@gmail.com","threadId":"56307","inReplyTo":"pull.1021.git.1629189701.gitgitgadget@gmail.com","subject":"[PATCH 1/8] [GSOC] ref-filter: remove grab_oid() function","fromName":"ZheNing Hu via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-08-17T08:41:34Z","receivedAt":"2021-08-17T08:41:46Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"From: ZheNing Hu <adlternative@gmail.com>\n\nBecause \"atom_type == ATOM_OBJECTNAME\" implies the condition\nof `starts_with(name, \"objectname\")`, \"atom_type == ATOM_TREE\"\nimplies the condition of `starts_with(name, \"tree\")`, so the\ncheck for `starts_with(name, field)` in grab_oid() is redundant.\n\nRemove the grab_oid() from ref-filter, to reduce repeated check.\n\nMentored-by: Christian Couder <christian.couder@gmail.com>\nMentored-by: Hariom Verma <hariom18599@gmail.com>\nSigned-off-by: ZheNing Hu <adlternative@gmail.com>\n---\n ref-filter.c | 26 +++++++++-----------------\n 1 file changed, 9 insertions(+), 17 deletions(-)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 93ce2a6ef2e..e38046ca141 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1065,16 +1065,6 @@ static const char *do_grab_oid(const char *field, const struct object_id *oid,\n \t}\n }\n \n-static int grab_oid(const char *name, const char *field, const struct object_id *oid,\n-\t\t    struct atom_value *v, struct used_atom *atom)\n-{\n-\tif (starts_with(name, field)) {\n-\t\tv->s = xstrdup(do_grab_oid(field, oid, atom));\n-\t\treturn 1;\n-\t}\n-\treturn 0;\n-}\n-\n /* See grab_values */\n static void grab_common_values(struct atom_value *val, int deref, struct expand_data *oi)\n {\n@@ -1100,8 +1090,9 @@ static void grab_common_values(struct atom_value *val, int deref, struct expand_\n \t\t\t}\n \t\t} else if (atom_type == ATOM_DELTABASE)\n \t\t\tv->s = xstrdup(oid_to_hex(&oi->delta_base_oid));\n-\t\telse if (atom_type == ATOM_OBJECTNAME && deref)\n-\t\t\tgrab_oid(name, \"objectname\", &oi->oid, v, &used_atom[i]);\n+\t\telse if (atom_type == ATOM_OBJECTNAME && deref) {\n+\t\t\tv->s = xstrdup(do_grab_oid(\"objectname\", &oi->oid, &used_atom[i]));\n+\t\t}\n \t}\n }\n \n@@ -1142,9 +1133,10 @@ static void grab_commit_values(struct atom_value *val, int deref, struct object\n \t\t\tcontinue;\n \t\tif (deref)\n \t\t\tname++;\n-\t\tif (atom_type == ATOM_TREE &&\n-\t\t    grab_oid(name, \"tree\", get_commit_tree_oid(commit), v, &used_atom[i]))\n+\t\tif (atom_type == ATOM_TREE) {\n+\t\t\tv->s = xstrdup(do_grab_oid(\"tree\", get_commit_tree_oid(commit), &used_atom[i]));\n \t\t\tcontinue;\n+\t\t}\n \t\tif (atom_type == ATOM_NUMPARENT) {\n \t\t\tv->value = commit_list_count(commit->parents);\n \t\t\tv->s = xstrfmt(\"%lu\", (unsigned long)v->value);\n@@ -1917,9 +1909,9 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)\n \t\t\t\tv->s = xstrdup(buf + 1);\n \t\t\t}\n \t\t\tcontinue;\n-\t\t} else if (!deref && atom_type == ATOM_OBJECTNAME &&\n-\t\t\t   grab_oid(name, \"objectname\", &ref->objectname, v, atom)) {\n-\t\t\t\tcontinue;\n+\t\t} else if (!deref && atom_type == ATOM_OBJECTNAME) {\n+\t\t\t   v->s = xstrdup(do_grab_oid(\"objectname\", &ref->objectname, atom));\n+\t\t\t   continue;\n \t\t} else if (atom_type == ATOM_HEAD) {\n \t\t\tif (atom->u.head && !strcmp(ref->refname, atom->u.head))\n \t\t\t\tv->s = xstrdup(\"*\");\n-- \ngitgitgadget\n\n"},{"id":"432920","messageId":"9ff78d17f3839acd24df5439fac74ba5064579c8.1629189701.git.gitgitgadget@gmail.com","threadId":"56307","inReplyTo":"pull.1021.git.1629189701.gitgitgadget@gmail.com","subject":"[PATCH 2/8] [GSOC] ref-filter: merge two for loop in grab_person","fromName":"ZheNing Hu via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-08-17T08:41:35Z","receivedAt":"2021-08-17T08:41:47Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"From: ZheNing Hu <adlternative@gmail.com>\n\ngrab_person() uses string for atom type comparison, this is very\ninefficient. After the introduction of atom_type, we can use\natom_type to represent the atom type.\n\nSo modify the parameter type of grab_person() to atom_type,\nand take use of the characteristics of ATOM_AUTHOR ==\nATOM_AUTHORNAME - 1 == ATOM_AUTHOREMAIL - 2 ==\nATOM_AUTHORDATE - 3 to check which author atom type is.\nATOM_COMMITTER, ATOM_TAGGER are same too.\n\nAt the same time, merge the for loop which handles %(creator*) in\ngrab_person() into the previous for loop which handles %(author*),\n%(committer*),%(tagger*). This can reduce unnecessary traversal\nof the used_atom list.\n\nMentored-by: Christian Couder <christian.couder@gmail.com>\nMentored-by: Hariom Verma <hariom18599@gmail.com>\nSigned-off-by: ZheNing Hu <adlternative@gmail.com>\n---\n ref-filter.c | 57 +++++++++++++++++-----------------------------------\n 1 file changed, 18 insertions(+), 39 deletions(-)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex e38046ca141..592b8d9bd0a 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1275,63 +1275,42 @@ static void grab_date(const char *buf, struct atom_value *v, const char *atomnam\n }\n \n /* See grab_values */\n-static void grab_person(const char *who, struct atom_value *val, int deref, void *buf)\n+static void grab_person(enum atom_type type, struct atom_value *val, int deref, void *buf)\n {\n \tint i;\n+\tconst char *who = valid_atom[type].name;\n \tint wholen = strlen(who);\n \tconst char *wholine = NULL;\n \n \tfor (i = 0; i < used_atom_cnt; i++) {\n \t\tconst char *name = used_atom[i].name;\n+\t\tenum atom_type atom_type = used_atom[i].atom_type;\n \t\tstruct atom_value *v = &val[i];\n \t\tif (!!deref != (*name == '*'))\n \t\t\tcontinue;\n \t\tif (deref)\n \t\t\tname++;\n-\t\tif (strncmp(who, name, wholen))\n-\t\t\tcontinue;\n-\t\tif (name[wholen] != 0 &&\n-\t\t    strcmp(name + wholen, \"name\") &&\n-\t\t    !starts_with(name + wholen, \"email\") &&\n-\t\t    !starts_with(name + wholen, \"date\"))\n+\t\tif ((atom_type < type || atom_type > type + 3) &&\n+\t\t    /*\n+\t\t    * For a tag or a commit object, if \"creator\" or \"creatordate\" is\n+\t\t    * requested, do something special.\n+\t\t    */\n+\t\t    ((atom_type != ATOM_CREATOR && atom_type != ATOM_CREATORDATE) ||\n+\t\t     ((atom_type == ATOM_CREATOR || atom_type == ATOM_CREATORDATE) &&\n+\t\t      type != ATOM_TAGGER && type != ATOM_COMMITTER)))\n \t\t\tcontinue;\n \t\tif (!wholine)\n \t\t\twholine = find_wholine(who, wholen, buf);\n \t\tif (!wholine)\n \t\t\treturn; /* no point looking for it */\n-\t\tif (name[wholen] == 0)\n+\t\tif (atom_type == type || atom_type == ATOM_CREATOR)\n \t\t\tv->s = copy_line(wholine);\n-\t\telse if (!strcmp(name + wholen, \"name\"))\n+\t\telse if (atom_type == type + 1)\n \t\t\tv->s = copy_name(wholine);\n-\t\telse if (starts_with(name + wholen, \"email\"))\n+\t\telse if (atom_type == type + 2)\n \t\t\tv->s = copy_email(wholine, &used_atom[i]);\n-\t\telse if (starts_with(name + wholen, \"date\"))\n-\t\t\tgrab_date(wholine, v, name);\n-\t}\n-\n-\t/*\n-\t * For a tag or a commit object, if \"creator\" or \"creatordate\" is\n-\t * requested, do something special.\n-\t */\n-\tif (strcmp(who, \"tagger\") && strcmp(who, \"committer\"))\n-\t\treturn; /* \"author\" for commit object is not wanted */\n-\tif (!wholine)\n-\t\twholine = find_wholine(who, wholen, buf);\n-\tif (!wholine)\n-\t\treturn;\n-\tfor (i = 0; i < used_atom_cnt; i++) {\n-\t\tconst char *name = used_atom[i].name;\n-\t\tenum atom_type atom_type = used_atom[i].atom_type;\n-\t\tstruct atom_value *v = &val[i];\n-\t\tif (!!deref != (*name == '*'))\n-\t\t\tcontinue;\n-\t\tif (deref)\n-\t\t\tname++;\n-\n-\t\tif (atom_type == ATOM_CREATORDATE)\n+\t\telse if (atom_type == type + 3 || atom_type == ATOM_CREATORDATE)\n \t\t\tgrab_date(wholine, v, name);\n-\t\telse if (atom_type == ATOM_CREATOR)\n-\t\t\tv->s = copy_line(wholine);\n \t}\n }\n \n@@ -1520,13 +1499,13 @@ static void grab_values(struct atom_value *val, int deref, struct object *obj, s\n \tcase OBJ_TAG:\n \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\tgrab_person(ATOM_TAGGER, val, deref, buf);\n \t\tbreak;\n \tcase OBJ_COMMIT:\n \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+\t\tgrab_person(ATOM_AUTHOR, val, deref, buf);\n+\t\tgrab_person(ATOM_COMMITTER, val, deref, buf);\n \t\tbreak;\n \tcase OBJ_TREE:\n \t\t/* grab_tree_values(val, deref, obj, buf, sz); */\n-- \ngitgitgadget\n\n"},{"id":"432921","messageId":"c3a4c858df0d246e06c3e770c5eb2956eeb96538.1629189701.git.gitgitgadget@gmail.com","threadId":"56307","inReplyTo":"pull.1021.git.1629189701.gitgitgadget@gmail.com","subject":"[PATCH 3/8] [GSOC] ref-filter: remove strlen from find_subpos","fromName":"ZheNing Hu via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-08-17T08:41:36Z","receivedAt":"2021-08-17T08:41:48Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"From: ZheNing Hu <adlternative@gmail.com>\n\nfind_subpos() use strlen() to get the length of the object\ncontent, but in the caller of find_subpos(), grab_sub_body_contents(),\nwe already get the length of the object content through data->size.\nSo add a `size_t buf_size` in find_subpos(), and pass data->size to\nit. This can avoid unnecessary strlen() overhead.\n\nSigned-off-by: ZheNing Hu <adlternative@gmail.com>\nMentored-by: Christian Couder <christian.couder@gmail.com>\nMentored-by: Hariom Verma <hariom18599@gmail.com>\n---\n ref-filter.c | 12 ++++++------\n 1 file changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 592b8d9bd0a..875c2e4c39c 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1314,7 +1314,7 @@ static void grab_person(enum atom_type type, struct atom_value *val, int deref,\n \t}\n }\n \n-static void find_subpos(const char *buf,\n+static void find_subpos(const char *buf, size_t buf_size,\n \t\t\tconst char **sub, size_t *sublen,\n \t\t\tconst char **body, size_t *bodylen,\n \t\t\tsize_t *nonsiglen,\n@@ -1322,12 +1322,12 @@ static void find_subpos(const char *buf,\n {\n \tstruct strbuf payload = STRBUF_INIT;\n \tstruct strbuf signature = STRBUF_INIT;\n+\tconst char *begin = buf;\n \tconst char *eol;\n-\tconst char *end = buf + strlen(buf);\n \tconst char *sigstart;\n \n \t/* parse signature first; we might not even have a subject line */\n-\tparse_signature(buf, end - buf, &payload, &signature);\n+\tparse_signature(buf, buf_size, &payload, &signature);\n \n \t/* skip past header until we hit empty line */\n \twhile (*buf && *buf != '\\n') {\n@@ -1340,7 +1340,7 @@ static void find_subpos(const char *buf,\n \twhile (*buf == '\\n')\n \t\tbuf++;\n \t*sig = strbuf_detach(&signature, siglen);\n-\tsigstart = buf + parse_signed_buffer(buf, strlen(buf));\n+\tsigstart = buf + parse_signed_buffer(buf, buf_size - (buf - begin));\n \n \t/* subject is first non-empty line */\n \t*sub = buf;\n@@ -1363,7 +1363,7 @@ static void find_subpos(const char *buf,\n \twhile (*buf == '\\n' || *buf == '\\r')\n \t\tbuf++;\n \t*body = buf;\n-\t*bodylen = strlen(buf);\n+\t*bodylen = buf_size - (buf - begin);\n \t*nonsiglen = sigstart - buf;\n }\n \n@@ -1430,7 +1430,7 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, struct exp\n \t\t     !starts_with(name, \"contents\")))\n \t\t\tcontinue;\n \t\tif (!subpos)\n-\t\t\tfind_subpos(buf,\n+\t\t\tfind_subpos(buf, data->size,\n \t\t\t\t    &subpos, &sublen,\n \t\t\t\t    &bodypos, &bodylen, &nonsiglen,\n \t\t\t\t    &sigpos, &siglen);\n-- \ngitgitgadget\n\n"},{"id":"432922","messageId":"38a36881f0a8db983be46c0c2bfe085d28225748.1629189701.git.gitgitgadget@gmail.com","threadId":"56307","inReplyTo":"pull.1021.git.1629189701.gitgitgadget@gmail.com","subject":"[PATCH 4/8] [GSOC] ref-filter: introducing xstrvfmt_len() and xstrfmt_len()","fromName":"ZheNing Hu via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-08-17T08:41:37Z","receivedAt":"2021-08-17T08:41:49Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"From: ZheNing Hu <adlternative@gmail.com>\n\nIn the logic of ref-filter, if we already specified v->s_size (the\nlength of the data), we can avoid using strlen() to calculate the\nlength of the data in strbuf_addstr() or perl_quote_buf_with_len()...\n\nxstrfmt(), xstrvfmt(), they are functions used to add format strings\nto a dynamic allocated buffer. Because their implementation uses strbuf\nto store its buffer and its length, so we can easily pass its length\nto the caller, furthermore, in ref-filter, we can reduce the use of\nstrlen().\n\nSo introduce xstrvfmt_len() and xstrfmt_len() which can fetch the length\nof the buffer through the parameter `len`. This can bring a slight\nperformance improvement.\n\nMentored-by: Christian Couder <christian.couder@gmail.com>\nMentored-by: Hariom Verma <hariom18599@gmail.com>\nSigned-off-by: ZheNing Hu <adlternative@gmail.com>\n---\n ref-filter.c | 73 ++++++++++++++++++++++++++++++++++------------------\n 1 file changed, 48 insertions(+), 25 deletions(-)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 875c2e4c39c..3784c0af57c 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -228,6 +228,29 @@ static int strbuf_addf_ret(struct strbuf *sb, int ret, const char *fmt, ...)\n \treturn ret;\n }\n \n+__attribute__((format (printf, 2, 0)))\n+static char *xstrvfmt_len(ssize_t *len, const char *fmt, va_list ap)\n+{\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tstrbuf_vaddf(&buf, fmt, ap);\n+\tif (len)\n+\t\t*len = buf.len;\n+\treturn strbuf_detach(&buf, NULL);\n+}\n+\n+__attribute__((format (printf, 2, 3)))\n+static  char *xstrfmt_len(ssize_t *len, const char *fmt, ...)\n+{\n+\tva_list ap;\n+\tchar *ret;\n+\n+\tva_start(ap, fmt);\n+\tret = xstrvfmt_len(len ,fmt, ap);\n+\tva_end(ap);\n+\n+\treturn ret;\n+}\n+\n static int color_atom_parser(struct ref_format *format, struct used_atom *atom,\n \t\t\t     const char *color_value, struct strbuf *err)\n {\n@@ -1083,10 +1106,10 @@ static void grab_common_values(struct atom_value *val, int deref, struct expand_\n \t\telse if (atom_type == ATOM_OBJECTSIZE) {\n \t\t\tif (used_atom[i].u.objectsize.option == O_SIZE_DISK) {\n \t\t\t\tv->value = oi->disk_size;\n-\t\t\t\tv->s = xstrfmt(\"%\"PRIuMAX, (uintmax_t)oi->disk_size);\n+\t\t\t\tv->s = xstrfmt_len(&v->s_size, \"%\"PRIuMAX, (uintmax_t)oi->disk_size);\n \t\t\t} else if (used_atom[i].u.objectsize.option == O_SIZE) {\n \t\t\t\tv->value = oi->size;\n-\t\t\t\tv->s = xstrfmt(\"%\"PRIuMAX , (uintmax_t)oi->size);\n+\t\t\t\tv->s = xstrfmt_len(&v->s_size, \"%\"PRIuMAX , (uintmax_t)oi->size);\n \t\t\t}\n \t\t} else if (atom_type == ATOM_DELTABASE)\n \t\t\tv->s = xstrdup(oid_to_hex(&oi->delta_base_oid));\n@@ -1139,7 +1162,7 @@ static void grab_commit_values(struct atom_value *val, int deref, struct object\n \t\t}\n \t\tif (atom_type == ATOM_NUMPARENT) {\n \t\t\tv->value = commit_list_count(commit->parents);\n-\t\t\tv->s = xstrfmt(\"%lu\", (unsigned long)v->value);\n+\t\t\tv->s = xstrfmt_len(&v->s_size, \"%lu\", (unsigned long)v->value);\n \t\t}\n \t\telse if (atom_type == ATOM_PARENT) {\n \t\t\tstruct commit_list *parents;\n@@ -1417,7 +1440,7 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, struct exp\n \t\t\t\tv->s = xmemdupz(buf, buf_size);\n \t\t\t\tv->s_size = buf_size;\n \t\t\t} else if (atom->u.raw_data.option == RAW_LENGTH) {\n-\t\t\t\tv->s = xstrfmt(\"%\"PRIuMAX, (uintmax_t)buf_size);\n+\t\t\t\tv->s = xstrfmt_len(&v->s_size, \"%\"PRIuMAX, (uintmax_t)buf_size);\n \t\t\t}\n \t\t\tcontinue;\n \t\t}\n@@ -1444,7 +1467,7 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, struct exp\n \t\t} else if (atom->u.contents.option == C_BODY_DEP)\n \t\t\tv->s = xmemdupz(bodypos, bodylen);\n \t\telse if (atom->u.contents.option == C_LENGTH)\n-\t\t\tv->s = xstrfmt(\"%\"PRIuMAX, (uintmax_t)strlen(subpos));\n+\t\t\tv->s = xstrfmt_len(&v->s_size, \"%\"PRIuMAX, (uintmax_t)strlen(subpos));\n \t\telse if (atom->u.contents.option == C_BODY)\n \t\t\tv->s = xmemdupz(bodypos, nonsiglen);\n \t\telse if (atom->u.contents.option == C_SIG)\n@@ -1611,56 +1634,56 @@ static const char *show_ref(struct refname_atom *atom, const char *refname)\n }\n \n static void fill_remote_ref_details(struct used_atom *atom, const char *refname,\n-\t\t\t\t    struct branch *branch, const char **s)\n+\t\t\t\t    struct branch *branch, struct atom_value *v)\n {\n \tint num_ours, num_theirs;\n \tif (atom->u.remote_ref.option == RR_REF)\n-\t\t*s = show_ref(&atom->u.remote_ref.refname, refname);\n+\t\tv->s = show_ref(&atom->u.remote_ref.refname, refname);\n \telse if (atom->u.remote_ref.option == RR_TRACK) {\n \t\tif (stat_tracking_info(branch, &num_ours, &num_theirs,\n \t\t\t\t       NULL, atom->u.remote_ref.push,\n \t\t\t\t       AHEAD_BEHIND_FULL) < 0) {\n-\t\t\t*s = xstrdup(msgs.gone);\n+\t\t\tv->s = xstrdup(msgs.gone);\n \t\t} else if (!num_ours && !num_theirs)\n-\t\t\t*s = xstrdup(\"\");\n+\t\t\tv->s = xstrdup(\"\");\n \t\telse if (!num_ours)\n-\t\t\t*s = xstrfmt(msgs.behind, num_theirs);\n+\t\t\tv->s = xstrfmt_len(&v->s_size, msgs.behind, num_theirs);\n \t\telse if (!num_theirs)\n-\t\t\t*s = xstrfmt(msgs.ahead, num_ours);\n+\t\t\tv->s = xstrfmt_len(&v->s_size, msgs.ahead, num_ours);\n \t\telse\n-\t\t\t*s = xstrfmt(msgs.ahead_behind,\n+\t\t\tv->s= xstrfmt_len(&v->s_size, msgs.ahead_behind,\n \t\t\t\t     num_ours, num_theirs);\n-\t\tif (!atom->u.remote_ref.nobracket && *s[0]) {\n-\t\t\tconst char *to_free = *s;\n-\t\t\t*s = xstrfmt(\"[%s]\", *s);\n+\t\tif (!atom->u.remote_ref.nobracket && v->s[0]) {\n+\t\t\tconst char *to_free = v->s;\n+\t\t\tv->s = xstrfmt_len(&v->s_size, \"[%s]\", v->s);\n \t\t\tfree((void *)to_free);\n \t\t}\n \t} else if (atom->u.remote_ref.option == RR_TRACKSHORT) {\n \t\tif (stat_tracking_info(branch, &num_ours, &num_theirs,\n \t\t\t\t       NULL, atom->u.remote_ref.push,\n \t\t\t\t       AHEAD_BEHIND_FULL) < 0) {\n-\t\t\t*s = xstrdup(\"\");\n+\t\t\tv->s = xstrdup(\"\");\n \t\t\treturn;\n \t\t}\n \t\tif (!num_ours && !num_theirs)\n-\t\t\t*s = xstrdup(\"=\");\n+\t\t\tv->s = xstrdup(\"=\");\n \t\telse if (!num_ours)\n-\t\t\t*s = xstrdup(\"<\");\n+\t\t\tv->s = xstrdup(\"<\");\n \t\telse if (!num_theirs)\n-\t\t\t*s = xstrdup(\">\");\n+\t\t\tv->s = xstrdup(\">\");\n \t\telse\n-\t\t\t*s = xstrdup(\"<>\");\n+\t\t\tv->s = xstrdup(\"<>\");\n \t} else if (atom->u.remote_ref.option == RR_REMOTE_NAME) {\n \t\tint explicit;\n \t\tconst char *remote = atom->u.remote_ref.push ?\n \t\t\tpushremote_for_branch(branch, &explicit) :\n \t\t\tremote_for_branch(branch, &explicit);\n-\t\t*s = xstrdup(explicit ? remote : \"\");\n+\t\tv->s = xstrdup(explicit ? remote : \"\");\n \t} else if (atom->u.remote_ref.option == RR_REMOTE_REF) {\n \t\tconst char *merge;\n \n \t\tmerge = remote_ref_for_branch(branch, atom->u.remote_ref.push);\n-\t\t*s = xstrdup(merge ? merge : \"\");\n+\t\tv->s = xstrdup(merge ? merge : \"\");\n \t} else\n \t\tBUG(\"unhandled RR_* enum\");\n }\n@@ -1849,7 +1872,7 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)\n \n \t\t\trefname = branch_get_upstream(branch, NULL);\n \t\t\tif (refname)\n-\t\t\t\tfill_remote_ref_details(atom, refname, branch, &v->s);\n+\t\t\t\tfill_remote_ref_details(atom, refname, branch, v);\n \t\t\telse\n \t\t\t\tv->s = xstrdup(\"\");\n \t\t\tcontinue;\n@@ -1870,7 +1893,7 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)\n \t\t\t}\n \t\t\t/* We will definitely re-init v->s on the next line. */\n \t\t\tfree((char *)v->s);\n-\t\t\tfill_remote_ref_details(atom, refname, branch, &v->s);\n+\t\t\tfill_remote_ref_details(atom, refname, branch, v);\n \t\t\tcontinue;\n \t\t} else if (atom_type == ATOM_COLOR) {\n \t\t\tv->s = xstrdup(atom->u.color);\n@@ -1933,7 +1956,7 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)\n \t\tif (!deref)\n \t\t\tv->s = xstrdup(refname);\n \t\telse\n-\t\t\tv->s = xstrfmt(\"%s^{}\", refname);\n+\t\t\tv->s = xstrfmt_len(&v->s_size, \"%s^{}\", refname);\n \t\tfree((char *)refname);\n \t}\n \n-- \ngitgitgadget\n\n"},{"id":"432923","messageId":"296dd2ae0119b9f465a534019089cd330afe5a2a.1629189701.git.gitgitgadget@gmail.com","threadId":"56307","inReplyTo":"pull.1021.git.1629189701.gitgitgadget@gmail.com","subject":"[PATCH 5/8] [GSOC] ref-filter: introduction ref_filter_slopbuf[1]","fromName":"ZheNing Hu via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-08-17T08:41:38Z","receivedAt":"2021-08-17T08:41:50Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"From: ZheNing Hu <adlternative@gmail.com>\n\nThe ref-filter code uses xstrdup(\"\") extensively to indicate null\nvalue.\n\nxstrdup(\"\") calls strdup(\"\"), which in glibc will call malloc(1),\nthis means using xstrdup(\"\") will cause unnecessary memory allocation.\n\nIntroduce the global one-byte array variable ref_filter_slopbuf[1],\nwhich learn from strbuf_slopbuf[1] in strbuf.c. Using ref_filter_slopbuf\ninstead of xstrdup(\"\"), we can reduce some malloc/free overhead.\n\nMentored-by: Christian Couder <christian.couder@gmail.com>\nMentored-by: Hariom Verma <hariom18599@gmail.com>\nSigned-off-by: ZheNing Hu <adlternative@gmail.com>\n---\n ref-filter.c | 53 ++++++++++++++++++++++++++--------------------------\n 1 file changed, 27 insertions(+), 26 deletions(-)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 3784c0af57c..b74fcbb4806 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -24,6 +24,8 @@\n #include \"hashmap.h\"\n #include \"strvec.h\"\n \n+static char ref_filter_slopbuf[1];\n+\n static struct ref_msg {\n \tconst char *gone;\n \tconst char *ahead;\n@@ -1209,7 +1211,7 @@ static const char *copy_name(const char *buf)\n \t\tif (!strncmp(cp, \" <\", 2))\n \t\t\treturn xmemdupz(buf, cp - buf);\n \t}\n-\treturn xstrdup(\"\");\n+\treturn ref_filter_slopbuf;\n }\n \n static const char *copy_email(const char *buf, struct used_atom *atom)\n@@ -1217,7 +1219,7 @@ static const char *copy_email(const char *buf, struct used_atom *atom)\n \tconst char *email = strchr(buf, '<');\n \tconst char *eoemail;\n \tif (!email)\n-\t\treturn xstrdup(\"\");\n+\t\treturn ref_filter_slopbuf;\n \tswitch (atom->u.email_option.option) {\n \tcase EO_RAW:\n \t\teoemail = strchr(email, '>');\n@@ -1239,7 +1241,7 @@ static const char *copy_email(const char *buf, struct used_atom *atom)\n \t}\n \n \tif (!eoemail)\n-\t\treturn xstrdup(\"\");\n+\t\treturn ref_filter_slopbuf;\n \treturn xmemdupz(email, eoemail - email);\n }\n \n@@ -1293,7 +1295,7 @@ static void grab_date(const char *buf, struct atom_value *v, const char *atomnam\n \tv->value = timestamp;\n \treturn;\n  bad:\n-\tv->s = xstrdup(\"\");\n+\tv->s = ref_filter_slopbuf;\n \tv->value = 0;\n }\n \n@@ -1503,7 +1505,7 @@ static void fill_missing_values(struct atom_value *val)\n \tfor (i = 0; i < used_atom_cnt; i++) {\n \t\tstruct atom_value *v = &val[i];\n \t\tif (v->s == NULL)\n-\t\t\tv->s = xstrdup(\"\");\n+\t\t\tv->s = ref_filter_slopbuf;\n \t}\n }\n \n@@ -1576,7 +1578,7 @@ static const char *lstrip_ref_components(const char *refname, int len)\n \t\tswitch (*start++) {\n \t\tcase '\\0':\n \t\t\tfree((char *)to_free);\n-\t\t\treturn xstrdup(\"\");\n+\t\t\treturn ref_filter_slopbuf;\n \t\tcase '/':\n \t\t\tremaining--;\n \t\t\tbreak;\n@@ -1614,7 +1616,7 @@ static const char *rstrip_ref_components(const char *refname, int len)\n \t\tchar *p = strrchr(start, '/');\n \t\tif (p == NULL) {\n \t\t\tfree((char *)to_free);\n-\t\t\treturn xstrdup(\"\");\n+\t\t\treturn ref_filter_slopbuf;\n \t\t} else\n \t\t\tp[0] = '\\0';\n \t}\n@@ -1645,7 +1647,7 @@ static void fill_remote_ref_details(struct used_atom *atom, const char *refname,\n \t\t\t\t       AHEAD_BEHIND_FULL) < 0) {\n \t\t\tv->s = xstrdup(msgs.gone);\n \t\t} else if (!num_ours && !num_theirs)\n-\t\t\tv->s = xstrdup(\"\");\n+\t\t\tv->s = ref_filter_slopbuf;\n \t\telse if (!num_ours)\n \t\t\tv->s = xstrfmt_len(&v->s_size, msgs.behind, num_theirs);\n \t\telse if (!num_theirs)\n@@ -1662,7 +1664,7 @@ static void fill_remote_ref_details(struct used_atom *atom, const char *refname,\n \t\tif (stat_tracking_info(branch, &num_ours, &num_theirs,\n \t\t\t\t       NULL, atom->u.remote_ref.push,\n \t\t\t\t       AHEAD_BEHIND_FULL) < 0) {\n-\t\t\tv->s = xstrdup(\"\");\n+\t\t\tv->s = ref_filter_slopbuf;\n \t\t\treturn;\n \t\t}\n \t\tif (!num_ours && !num_theirs)\n@@ -1721,7 +1723,7 @@ char *get_head_description(void)\n static const char *get_symref(struct used_atom *atom, struct ref_array_item *ref)\n {\n \tif (!ref->symref)\n-\t\treturn xstrdup(\"\");\n+\t\treturn ref_filter_slopbuf;\n \telse\n \t\treturn show_ref(&atom->u.refname, ref->symref);\n }\n@@ -1805,7 +1807,7 @@ static char *get_worktree_path(const struct used_atom *atom, const struct ref_ar\n \te = hashmap_get(&(ref_to_worktree_map.map), &entry, ref->refname);\n \n \tif (!e)\n-\t\treturn xstrdup(\"\");\n+\t\treturn ref_filter_slopbuf;\n \n \tlookup_result = container_of(e, struct ref_to_worktree_entry, ent);\n \n@@ -1827,7 +1829,7 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)\n \t\tref->symref = resolve_refdup(ref->refname, RESOLVE_REF_READING,\n \t\t\t\t\t     NULL, NULL);\n \t\tif (!ref->symref)\n-\t\t\tref->symref = xstrdup(\"\");\n+\t\t\tref->symref = ref_filter_slopbuf;\n \t}\n \n \t/* Fill in specials first */\n@@ -1855,7 +1857,7 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)\n \t\t\tif (ref->kind == FILTER_REFS_BRANCHES)\n \t\t\t\tv->s = get_worktree_path(atom, ref);\n \t\t\telse\n-\t\t\t\tv->s = xstrdup(\"\");\n+\t\t\t\tv->s = ref_filter_slopbuf;\n \t\t\tcontinue;\n \t\t}\n \t\telse if (atom_type == ATOM_SYMREF)\n@@ -1865,7 +1867,7 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)\n \t\t\t/* only local branches may have an upstream */\n \t\t\tif (!skip_prefix(ref->refname, \"refs/heads/\",\n \t\t\t\t\t &branch_name)) {\n-\t\t\t\tv->s = xstrdup(\"\");\n+\t\t\t\tv->s = ref_filter_slopbuf;\n \t\t\t\tcontinue;\n \t\t\t}\n \t\t\tbranch = branch_get(branch_name);\n@@ -1874,11 +1876,11 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)\n \t\t\tif (refname)\n \t\t\t\tfill_remote_ref_details(atom, refname, branch, v);\n \t\t\telse\n-\t\t\t\tv->s = xstrdup(\"\");\n+\t\t\t\tv->s = ref_filter_slopbuf;\n \t\t\tcontinue;\n \t\t} else if (atom_type == ATOM_PUSH && atom->u.remote_ref.push) {\n \t\t\tconst char *branch_name;\n-\t\t\tv->s = xstrdup(\"\");\n+\t\t\tv->s = ref_filter_slopbuf;\n \t\t\tif (!skip_prefix(ref->refname, \"refs/heads/\",\n \t\t\t\t\t &branch_name))\n \t\t\t\tcontinue;\n@@ -1891,8 +1893,6 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)\n \t\t\t\tif (!refname)\n \t\t\t\t\tcontinue;\n \t\t\t}\n-\t\t\t/* We will definitely re-init v->s on the next line. */\n-\t\t\tfree((char *)v->s);\n \t\t\tfill_remote_ref_details(atom, refname, branch, v);\n \t\t\tcontinue;\n \t\t} else if (atom_type == ATOM_COLOR) {\n@@ -1905,7 +1905,7 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)\n \t\t\tif (ref->flag & REF_ISPACKED)\n \t\t\t\tcp = copy_advance(cp, \",packed\");\n \t\t\tif (cp == buf)\n-\t\t\t\tv->s = xstrdup(\"\");\n+\t\t\t\tv->s = ref_filter_slopbuf;\n \t\t\telse {\n \t\t\t\t*cp = '\\0';\n \t\t\t\tv->s = xstrdup(buf + 1);\n@@ -1922,33 +1922,33 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)\n \t\t\tcontinue;\n \t\t} else if (atom_type == ATOM_ALIGN) {\n \t\t\tv->handler = align_atom_handler;\n-\t\t\tv->s = xstrdup(\"\");\n+\t\t\tv->s = ref_filter_slopbuf;\n \t\t\tcontinue;\n \t\t} else if (atom_type == ATOM_END) {\n \t\t\tv->handler = end_atom_handler;\n-\t\t\tv->s = xstrdup(\"\");\n+\t\t\tv->s = ref_filter_slopbuf;\n \t\t\tcontinue;\n \t\t} else if (atom_type == ATOM_IF) {\n \t\t\tconst char *s;\n \t\t\tif (skip_prefix(name, \"if:\", &s))\n \t\t\t\tv->s = xstrdup(s);\n \t\t\telse\n-\t\t\t\tv->s = xstrdup(\"\");\n+\t\t\t\tv->s = ref_filter_slopbuf;\n \t\t\tv->handler = if_atom_handler;\n \t\t\tcontinue;\n \t\t} else if (atom_type == ATOM_THEN) {\n \t\t\tv->handler = then_atom_handler;\n-\t\t\tv->s = xstrdup(\"\");\n+\t\t\tv->s = ref_filter_slopbuf;\n \t\t\tcontinue;\n \t\t} else if (atom_type == ATOM_ELSE) {\n \t\t\tv->handler = else_atom_handler;\n-\t\t\tv->s = xstrdup(\"\");\n+\t\t\tv->s = ref_filter_slopbuf;\n \t\t\tcontinue;\n \t\t} else if (atom_type == ATOM_REST) {\n \t\t\tif (ref->rest)\n \t\t\t\tv->s = xstrdup(ref->rest);\n \t\t\telse\n-\t\t\t\tv->s = xstrdup(\"\");\n+\t\t\t\tv->s = ref_filter_slopbuf;\n \t\t\tcontinue;\n \t\t} else\n \t\t\tcontinue;\n@@ -2297,7 +2297,8 @@ static void free_array_item(struct ref_array_item *item)\n \tif (item->value) {\n \t\tint i;\n \t\tfor (i = 0; i < used_atom_cnt; i++)\n-\t\t\tfree((char *)item->value[i].s);\n+\t\t\tif (item->value[i].s != ref_filter_slopbuf)\n+\t\t\t\tfree((char *)item->value[i].s);\n \t\tfree(item->value);\n \t}\n \tfree(item);\n-- \ngitgitgadget\n\n"},{"id":"432924","messageId":"33b227e5fdf60c28392ef9660e9ae6f149a905bf.1629189701.git.gitgitgadget@gmail.com","threadId":"56307","inReplyTo":"pull.1021.git.1629189701.gitgitgadget@gmail.com","subject":"[PATCH 6/8] [GSOC] ref-filter: add deref member to struct used_atom","fromName":"ZheNing Hu via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-08-17T08:41:39Z","receivedAt":"2021-08-17T08:41:53Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"From: ZheNing Hu <adlternative@gmail.com>\n\nIn the ref-filter logic, the dereference flag '*' is stored\nat the beginning of used_atom[i].name (e.g. '*objectname:short'),\nthis leads us to make some check on used_atom[i].name[0] to\nknow if the atom need to be dereferenced. This is not only\ncumbersome to use, but also poor in readability.\n\nSo add `deref` flag to struct used_atom used to indicate\nwhether the atom needs to be dereferenced and let used_atom\nrecord only the rest part (i.e. 'objectname:short').\n\nThis can make the logic of the program clearer and easy to use.\n\nMentored-by: Christian Couder <christian.couder@gmail.com>\nMentored-by: Hariom Verma <hariom18599@gmail.com>\nSigned-off-by: ZheNing Hu <adlternative@gmail.com>\n---\n ref-filter.c | 55 ++++++++++++++++++++--------------------------------\n 1 file changed, 21 insertions(+), 34 deletions(-)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex b74fcbb4806..5abcbbf10ac 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -173,6 +173,7 @@ enum atom_type {\n  * array.\n  */\n static struct used_atom {\n+\tunsigned int deref : 1;\n \tenum atom_type atom_type;\n \tconst char *name;\n \tcmp_type type;\n@@ -343,7 +344,7 @@ 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->deref)\n \t\toi_deref.info.typep = &oi_deref.type;\n \telse\n \t\toi.info.typep = &oi.type;\n@@ -355,13 +356,13 @@ 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->deref)\n \t\t\toi_deref.info.sizep = &oi_deref.size;\n \t\telse\n \t\t\toi.info.sizep = &oi.size;\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->deref)\n \t\t\toi_deref.info.disk_sizep = &oi_deref.disk_size;\n \t\telse\n \t\t\toi.info.disk_sizep = &oi.disk_size;\n@@ -375,7 +376,7 @@ static int deltabase_atom_parser(struct ref_format *format, struct used_atom *at\n {\n \tif (arg)\n \t\treturn strbuf_addf_ret(err, -1, _(\"%%(deltabase) does not take arguments\"));\n-\tif (*atom->name == '*')\n+\tif (atom->deref)\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@@ -697,10 +698,13 @@ static int parse_ref_filter_atom(struct ref_format *format,\n \tconst char *sp;\n \tconst char *arg;\n \tint i, at, atom_len;\n+\tint deref = 0;\n \n \tsp = atom;\n-\tif (*sp == '*' && sp < ep)\n+\tif (*sp == '*' && sp < ep) {\n \t\tsp++; /* deref */\n+\t\tderef = 1;\n+\t}\n \tif (ep <= sp)\n \t\treturn strbuf_addf_ret(err, -1, _(\"malformed field name: %.*s\"),\n \t\t\t\t       (int)(ep-atom), atom);\n@@ -717,7 +721,7 @@ static int parse_ref_filter_atom(struct ref_format *format,\n \t/* Do we have the atom already used elsewhere? */\n \tfor (i = 0; i < used_atom_cnt; i++) {\n \t\tint len = strlen(used_atom[i].name);\n-\t\tif (len == ep - atom && !memcmp(used_atom[i].name, atom, len))\n+\t\tif (len == ep - sp && !memcmp(used_atom[i].name, sp, len))\n \t\t\treturn i;\n \t}\n \n@@ -741,17 +745,18 @@ static int parse_ref_filter_atom(struct ref_format *format,\n \tused_atom_cnt++;\n \tREALLOC_ARRAY(used_atom, used_atom_cnt);\n \tused_atom[at].atom_type = i;\n-\tused_atom[at].name = xmemdupz(atom, ep - atom);\n+\tused_atom[at].deref = deref;\n+\tused_atom[at].name = xmemdupz(sp, ep - sp);\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\tif (*atom == '*')\n+\t\tif (deref)\n \t\t\toi_deref.info.contentp = &oi_deref.content;\n \t\telse\n \t\t\toi.info.contentp = &oi.content;\n \t}\n \tif (arg) {\n-\t\targ = used_atom[at].name + (arg - atom) + 1;\n+\t\targ = used_atom[at].name + (arg - sp) + 1;\n \t\tif (!*arg) {\n \t\t\t/*\n \t\t\t * Treat empty sub-arguments list as NULL (i.e.,\n@@ -763,7 +768,7 @@ static int parse_ref_filter_atom(struct ref_format *format,\n \tmemset(&used_atom[at].u, 0, sizeof(used_atom[at].u));\n \tif (valid_atom[i].parser && valid_atom[i].parser(format, &used_atom[at], arg, err))\n \t\treturn -1;\n-\tif (*atom == '*')\n+\tif (deref)\n \t\tneed_tagged = 1;\n \tif (i == ATOM_SYMREF)\n \t\tneed_symref = 1;\n@@ -1096,13 +1101,10 @@ static void grab_common_values(struct atom_value *val, int deref, struct expand_\n \tint i;\n \n \tfor (i = 0; i < used_atom_cnt; i++) {\n-\t\tconst char *name = used_atom[i].name;\n \t\tenum atom_type atom_type = used_atom[i].atom_type;\n \t\tstruct atom_value *v = &val[i];\n-\t\tif (!!deref != (*name == '*'))\n+\t\tif (!!deref != used_atom[i].deref)\n \t\t\tcontinue;\n-\t\tif (deref)\n-\t\t\tname++;\n \t\tif (atom_type == ATOM_OBJECTTYPE)\n \t\t\tv->s = xstrdup(type_name(oi->type));\n \t\telse if (atom_type == ATOM_OBJECTSIZE) {\n@@ -1128,13 +1130,10 @@ static void grab_tag_values(struct atom_value *val, int deref, struct object *ob\n \tstruct tag *tag = (struct tag *) obj;\n \n \tfor (i = 0; i < used_atom_cnt; i++) {\n-\t\tconst char *name = used_atom[i].name;\n \t\tenum atom_type atom_type = used_atom[i].atom_type;\n \t\tstruct atom_value *v = &val[i];\n-\t\tif (!!deref != (*name == '*'))\n+\t\tif (!!deref != used_atom[i].deref)\n \t\t\tcontinue;\n-\t\tif (deref)\n-\t\t\tname++;\n \t\tif (atom_type == ATOM_TAG)\n \t\t\tv->s = xstrdup(tag->tag);\n \t\telse if (atom_type == ATOM_TYPE && tag->tagged)\n@@ -1151,13 +1150,10 @@ static void grab_commit_values(struct atom_value *val, int deref, struct object\n \tstruct commit *commit = (struct commit *) obj;\n \n \tfor (i = 0; i < used_atom_cnt; i++) {\n-\t\tconst char *name = used_atom[i].name;\n \t\tenum atom_type atom_type = used_atom[i].atom_type;\n \t\tstruct atom_value *v = &val[i];\n-\t\tif (!!deref != (*name == '*'))\n+\t\tif (!!deref != used_atom[i].deref)\n \t\t\tcontinue;\n-\t\tif (deref)\n-\t\t\tname++;\n \t\tif (atom_type == ATOM_TREE) {\n \t\t\tv->s = xstrdup(do_grab_oid(\"tree\", get_commit_tree_oid(commit), &used_atom[i]));\n \t\t\tcontinue;\n@@ -1311,10 +1307,8 @@ static void grab_person(enum atom_type type, struct atom_value *val, int deref,\n \t\tconst char *name = used_atom[i].name;\n \t\tenum atom_type atom_type = used_atom[i].atom_type;\n \t\tstruct atom_value *v = &val[i];\n-\t\tif (!!deref != (*name == '*'))\n+\t\tif (!!deref != used_atom[i].deref)\n \t\t\tcontinue;\n-\t\tif (deref)\n-\t\t\tname++;\n \t\tif ((atom_type < type || atom_type > type + 3) &&\n \t\t    /*\n \t\t    * For a tag or a commit object, if \"creator\" or \"creatordate\" is\n@@ -1430,10 +1424,8 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, struct exp\n \t\tstruct atom_value *v = &val[i];\n \t\tenum atom_type atom_type = atom->atom_type;\n \n-\t\tif (!!deref != (*name == '*'))\n+\t\tif (!!deref != used_atom[i].deref)\n \t\t\tcontinue;\n-\t\tif (deref)\n-\t\t\tname++;\n \n \t\tif (atom_type == ATOM_RAW) {\n \t\t\tunsigned long buf_size = data->size;\n@@ -1838,7 +1830,7 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)\n \t\tenum atom_type atom_type = atom->atom_type;\n \t\tconst char *name = used_atom[i].name;\n \t\tstruct atom_value *v = &ref->value[i];\n-\t\tint deref = 0;\n+\t\tint deref = atom->deref;\n \t\tconst char *refname;\n \t\tstruct branch *branch = NULL;\n \n@@ -1846,11 +1838,6 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)\n \t\tv->handler = append_atom;\n \t\tv->atom = atom;\n \n-\t\tif (*name == '*') {\n-\t\t\tderef = 1;\n-\t\t\tname++;\n-\t\t}\n-\n \t\tif (atom_type == ATOM_REFNAME)\n \t\t\trefname = get_refname(atom, ref);\n \t\telse if (atom_type == ATOM_WORKTREEPATH) {\n-- \ngitgitgadget\n\n"},{"id":"432925","messageId":"61d452a6278abfe84680a11c28cde9d3a48d0848.1629189701.git.gitgitgadget@gmail.com","threadId":"56307","inReplyTo":"pull.1021.git.1629189701.gitgitgadget@gmail.com","subject":"[PATCH 7/8] [GSOC] ref-filter: introduce symref_atom_parser()","fromName":"ZheNing Hu via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-08-17T08:41:40Z","receivedAt":"2021-08-17T08:41:54Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"From: ZheNing Hu <adlternative@gmail.com>\n\nparse_ref_filter_atom() sets need_symref by checking\nwhether the atom type is ATOM_SYMREF. When we are operating\nother atoms here, this step of checking is not necessary.\n\nSo add the symref_atom_parser() specifically to parse the\n%(symref) atom, and set need_symref in it.\n\nThis can make the program logic more concise.\n\nMentored-by: Christian Couder <christian.couder@gmail.com>\nMentored-by: Hariom Verma <hariom18599@gmail.com>\nSigned-off-by: ZheNing Hu <adlternative@gmail.com>\n---\n ref-filter.c | 11 ++++++++---\n 1 file changed, 8 insertions(+), 3 deletions(-)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 5abcbbf10ac..f1c82e20e3d 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -503,6 +503,13 @@ static int person_email_atom_parser(struct ref_format *format, struct used_atom\n \treturn 0;\n }\n \n+static int symref_atom_parser(struct ref_format *format, struct used_atom *atom,\n+\t\t\t       const char *arg, struct strbuf *err)\n+{\n+\tneed_symref = 1;\n+\treturn refname_atom_parser_internal(&atom->u.refname, arg, atom->name, err);\n+}\n+\n static int refname_atom_parser(struct ref_format *format, struct used_atom *atom,\n \t\t\t       const char *arg, struct strbuf *err)\n {\n@@ -642,7 +649,7 @@ static struct {\n \t[ATOM_RAW] = { \"raw\", SOURCE_OBJ, FIELD_STR, raw_atom_parser },\n \t[ATOM_UPSTREAM] = { \"upstream\", SOURCE_NONE, FIELD_STR, remote_ref_atom_parser },\n \t[ATOM_PUSH] = { \"push\", SOURCE_NONE, FIELD_STR, remote_ref_atom_parser },\n-\t[ATOM_SYMREF] = { \"symref\", SOURCE_NONE, FIELD_STR, refname_atom_parser },\n+\t[ATOM_SYMREF] = { \"symref\", SOURCE_NONE, FIELD_STR, symref_atom_parser },\n \t[ATOM_FLAG] = { \"flag\", SOURCE_NONE },\n \t[ATOM_HEAD] = { \"HEAD\", SOURCE_NONE, FIELD_STR, head_atom_parser },\n \t[ATOM_COLOR] = { \"color\", SOURCE_NONE, FIELD_STR, color_atom_parser },\n@@ -770,8 +777,6 @@ static int parse_ref_filter_atom(struct ref_format *format,\n \t\treturn -1;\n \tif (deref)\n \t\tneed_tagged = 1;\n-\tif (i == ATOM_SYMREF)\n-\t\tneed_symref = 1;\n \treturn at;\n }\n \n-- \ngitgitgadget\n\n"},{"id":"432926","messageId":"2321b873d0c0223e553492d80ced2a51d8ce7281.1629189701.git.gitgitgadget@gmail.com","threadId":"56307","inReplyTo":"pull.1021.git.1629189701.gitgitgadget@gmail.com","subject":"[PATCH 8/8] [GSOC] ref-filter: use switch/case instead of if/else","fromName":"ZheNing Hu via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-08-17T08:41:41Z","receivedAt":"2021-08-17T08:41:55Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"From: ZheNing Hu <adlternative@gmail.com>\n\nIn the logic of ref-filter, if/else is used extensively\nto check the atom type. This is because before we introduce\natom_type, the atom check is achieved through string comparison.\n\nUsing switch/case can allow the compiler to reduce unnecessary\nbranch checks and increase the readability of the code.\n\nMentored-by: Christian Couder <christian.couder@gmail.com>\nMentored-by: Hariom Verma <hariom18599@gmail.com>\nSigned-off-by: ZheNing Hu <adlternative@gmail.com>\n---\n ref-filter.c | 163 ++++++++++++++++++++++++++++++++++-----------------\n 1 file changed, 109 insertions(+), 54 deletions(-)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex f1c82e20e3d..c0b5d30ac57 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1110,9 +1110,11 @@ static void grab_common_values(struct atom_value *val, int deref, struct expand_\n \t\tstruct atom_value *v = &val[i];\n \t\tif (!!deref != used_atom[i].deref)\n \t\t\tcontinue;\n-\t\tif (atom_type == ATOM_OBJECTTYPE)\n+\t\tswitch (atom_type) {\n+\t\tcase ATOM_OBJECTTYPE : {\n \t\t\tv->s = xstrdup(type_name(oi->type));\n-\t\telse if (atom_type == ATOM_OBJECTSIZE) {\n+\t\t\tbreak;\n+\t\t} case ATOM_OBJECTSIZE : {\n \t\t\tif (used_atom[i].u.objectsize.option == O_SIZE_DISK) {\n \t\t\t\tv->value = oi->disk_size;\n \t\t\t\tv->s = xstrfmt_len(&v->s_size, \"%\"PRIuMAX, (uintmax_t)oi->disk_size);\n@@ -1120,10 +1122,20 @@ static void grab_common_values(struct atom_value *val, int deref, struct expand_\n \t\t\t\tv->value = oi->size;\n \t\t\t\tv->s = xstrfmt_len(&v->s_size, \"%\"PRIuMAX , (uintmax_t)oi->size);\n \t\t\t}\n-\t\t} else if (atom_type == ATOM_DELTABASE)\n+\t\t\tbreak;\n+\t\t}\n+\t\tcase ATOM_DELTABASE: {\n \t\t\tv->s = xstrdup(oid_to_hex(&oi->delta_base_oid));\n-\t\telse if (atom_type == ATOM_OBJECTNAME && deref) {\n-\t\t\tv->s = xstrdup(do_grab_oid(\"objectname\", &oi->oid, &used_atom[i]));\n+\t\t\tbreak;\n+\t\t}\n+\t\tcase ATOM_OBJECTNAME: {\n+\t\t\tif (deref)\n+\t\t\t\tv->s = xstrdup(do_grab_oid(\"objectname\", &oi->oid, &used_atom[i]));\n+\t\t\tbreak;\n+\t\t}\n+\t\tdefault: {\n+\t\t\tbreak;\n+\t\t}\n \t\t}\n \t}\n }\n@@ -1138,13 +1150,22 @@ static void grab_tag_values(struct atom_value *val, int deref, struct object *ob\n \t\tenum atom_type atom_type = used_atom[i].atom_type;\n \t\tstruct atom_value *v = &val[i];\n \t\tif (!!deref != used_atom[i].deref)\n-\t\t\tcontinue;\n-\t\tif (atom_type == ATOM_TAG)\n+\t\t\tbreak;\n+\t\tswitch (atom_type) {\n+\t\tcase ATOM_TAG:\n \t\t\tv->s = xstrdup(tag->tag);\n-\t\telse if (atom_type == ATOM_TYPE && tag->tagged)\n-\t\t\tv->s = xstrdup(type_name(tag->tagged->type));\n-\t\telse if (atom_type == ATOM_OBJECT && tag->tagged)\n-\t\t\tv->s = xstrdup(oid_to_hex(&tag->tagged->oid));\n+\t\t\tbreak;\n+\t\tcase ATOM_TYPE:\n+\t\t\tif (tag->tagged)\n+\t\t\t\tv->s = xstrdup(type_name(tag->tagged->type));\n+\t\t\tbreak;\n+\t\tcase ATOM_OBJECT:\n+\t\t\tif (tag->tagged)\n+\t\t\t\tv->s = xstrdup(oid_to_hex(&tag->tagged->oid));\n+\t\t\tbreak;\n+\t\tdefault:\n+\t\t\tbreak;\n+\t\t}\n \t}\n }\n \n@@ -1159,15 +1180,17 @@ static void grab_commit_values(struct atom_value *val, int deref, struct object\n \t\tstruct atom_value *v = &val[i];\n \t\tif (!!deref != used_atom[i].deref)\n \t\t\tcontinue;\n-\t\tif (atom_type == ATOM_TREE) {\n+\t\tswitch (atom_type) {\n+\t\tcase ATOM_TREE: {\n \t\t\tv->s = xstrdup(do_grab_oid(\"tree\", get_commit_tree_oid(commit), &used_atom[i]));\n-\t\t\tcontinue;\n+\t\t\tbreak;\n \t\t}\n-\t\tif (atom_type == ATOM_NUMPARENT) {\n+\t\tcase ATOM_NUMPARENT: {\n \t\t\tv->value = commit_list_count(commit->parents);\n \t\t\tv->s = xstrfmt_len(&v->s_size, \"%lu\", (unsigned long)v->value);\n+\t\t\tbreak;\n \t\t}\n-\t\telse if (atom_type == ATOM_PARENT) {\n+\t\tcase ATOM_PARENT: {\n \t\t\tstruct commit_list *parents;\n \t\t\tstruct strbuf s = STRBUF_INIT;\n \t\t\tfor (parents = commit->parents; parents; parents = parents->next) {\n@@ -1177,6 +1200,11 @@ static void grab_commit_values(struct atom_value *val, int deref, struct object\n \t\t\t\tstrbuf_addstr(&s, do_grab_oid(\"parent\", oid, &used_atom[i]));\n \t\t\t}\n \t\t\tv->s = strbuf_detach(&s, NULL);\n+\t\t\tbreak;\n+\t\t}\n+\t\tdefault: {\n+\t\t\tbreak;\n+\t\t}\n \t\t}\n \t}\n }\n@@ -1843,24 +1871,40 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)\n \t\tv->handler = append_atom;\n \t\tv->atom = atom;\n \n-\t\tif (atom_type == ATOM_REFNAME)\n+\t\tswitch (atom_type) {\n+\t\tcase ATOM_REFNAME: {\n \t\t\trefname = get_refname(atom, ref);\n-\t\telse if (atom_type == ATOM_WORKTREEPATH) {\n+\t\t\tif (!deref)\n+\t\t\t\tv->s = xstrdup(refname);\n+\t\t\telse\n+\t\t\t\tv->s = xstrfmt_len(&v->s_size, \"%s^{}\", refname);\n+\t\t\tfree((char *)refname);\n+\t\t\tbreak;\n+\t\t}\n+\t\tcase ATOM_WORKTREEPATH: {\n \t\t\tif (ref->kind == FILTER_REFS_BRANCHES)\n \t\t\t\tv->s = get_worktree_path(atom, ref);\n \t\t\telse\n \t\t\t\tv->s = ref_filter_slopbuf;\n-\t\t\tcontinue;\n+\t\t\tbreak;\n \t\t}\n-\t\telse if (atom_type == ATOM_SYMREF)\n+\t\tcase ATOM_SYMREF: {\n \t\t\trefname = get_symref(atom, ref);\n-\t\telse if (atom_type == ATOM_UPSTREAM) {\n+\t\t\tif (!deref)\n+\t\t\t\tv->s = xstrdup(refname);\n+\t\t\telse\n+\t\t\t\tv->s = xstrfmt_len(&v->s_size, \"%s^{}\", refname);\n+\t\t\tif (refname != ref_filter_slopbuf)\n+\t\t\t\tfree((char *)refname);\n+\t\t\tbreak;\n+\t\t}\n+\t\tcase ATOM_UPSTREAM: {\n \t\t\tconst char *branch_name;\n \t\t\t/* only local branches may have an upstream */\n \t\t\tif (!skip_prefix(ref->refname, \"refs/heads/\",\n \t\t\t\t\t &branch_name)) {\n \t\t\t\tv->s = ref_filter_slopbuf;\n-\t\t\t\tcontinue;\n+\t\t\tbreak;\n \t\t\t}\n \t\t\tbranch = branch_get(branch_name);\n \n@@ -1869,13 +1913,17 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)\n \t\t\t\tfill_remote_ref_details(atom, refname, branch, v);\n \t\t\telse\n \t\t\t\tv->s = ref_filter_slopbuf;\n-\t\t\tcontinue;\n-\t\t} else if (atom_type == ATOM_PUSH && atom->u.remote_ref.push) {\n+\t\t\tbreak;\n+\t\t}\n+\t\tcase ATOM_PUSH: {\n \t\t\tconst char *branch_name;\n+\n+\t\t\tif (!atom->u.remote_ref.push)\n+\t\t\t\tbreak;\n \t\t\tv->s = ref_filter_slopbuf;\n \t\t\tif (!skip_prefix(ref->refname, \"refs/heads/\",\n \t\t\t\t\t &branch_name))\n-\t\t\t\tcontinue;\n+\t\t\t\tbreak;\n \t\t\tbranch = branch_get(branch_name);\n \n \t\t\tif (atom->u.remote_ref.push_remote)\n@@ -1883,14 +1931,16 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)\n \t\t\telse {\n \t\t\t\trefname = branch_get_push(branch, NULL);\n \t\t\t\tif (!refname)\n-\t\t\t\t\tcontinue;\n+\t\t\t\t\tbreak;\n \t\t\t}\n \t\t\tfill_remote_ref_details(atom, refname, branch, v);\n-\t\t\tcontinue;\n-\t\t} else if (atom_type == ATOM_COLOR) {\n+\t\t\tbreak;\n+\t\t}\n+\t\tcase ATOM_COLOR: {\n \t\t\tv->s = xstrdup(atom->u.color);\n \t\t\tcontinue;\n-\t\t} else if (atom_type == ATOM_FLAG) {\n+\t\t}\n+\t\tcase ATOM_FLAG: {\n \t\t\tchar buf[256], *cp = buf;\n \t\t\tif (ref->flag & REF_ISSYMREF)\n \t\t\t\tcp = copy_advance(cp, \",symref\");\n@@ -1902,54 +1952,59 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)\n \t\t\t\t*cp = '\\0';\n \t\t\t\tv->s = xstrdup(buf + 1);\n \t\t\t}\n-\t\t\tcontinue;\n-\t\t} else if (!deref && atom_type == ATOM_OBJECTNAME) {\n-\t\t\t   v->s = xstrdup(do_grab_oid(\"objectname\", &ref->objectname, atom));\n-\t\t\t   continue;\n-\t\t} else if (atom_type == ATOM_HEAD) {\n+\t\t\tbreak;\n+\t\t}\n+\t\tcase ATOM_OBJECTNAME: {\n+\t\t\tif (!deref)\n+\t\t\t\tv->s = xstrdup(do_grab_oid(\"objectname\", &ref->objectname, atom));\n+\t\t\tbreak;\n+\t\t}\n+\t\tcase ATOM_HEAD: {\n \t\t\tif (atom->u.head && !strcmp(ref->refname, atom->u.head))\n \t\t\t\tv->s = xstrdup(\"*\");\n \t\t\telse\n \t\t\t\tv->s = xstrdup(\" \");\n-\t\t\tcontinue;\n-\t\t} else if (atom_type == ATOM_ALIGN) {\n+\t\t\tbreak;\n+\t\t}\n+\t\tcase ATOM_ALIGN: {\n \t\t\tv->handler = align_atom_handler;\n \t\t\tv->s = ref_filter_slopbuf;\n-\t\t\tcontinue;\n-\t\t} else if (atom_type == ATOM_END) {\n+\t\t\tbreak;\n+\t\t}\n+\t\tcase ATOM_END: {\n \t\t\tv->handler = end_atom_handler;\n \t\t\tv->s = ref_filter_slopbuf;\n-\t\t\tcontinue;\n-\t\t} else if (atom_type == ATOM_IF) {\n+\t\t\tbreak;\n+\t\t}\n+\t\tcase ATOM_IF: {\n \t\t\tconst char *s;\n \t\t\tif (skip_prefix(name, \"if:\", &s))\n \t\t\t\tv->s = xstrdup(s);\n \t\t\telse\n \t\t\t\tv->s = ref_filter_slopbuf;\n \t\t\tv->handler = if_atom_handler;\n-\t\t\tcontinue;\n-\t\t} else if (atom_type == ATOM_THEN) {\n+\t\t\tbreak;\n+\t\t}\n+\t\tcase ATOM_THEN: {\n \t\t\tv->handler = then_atom_handler;\n \t\t\tv->s = ref_filter_slopbuf;\n-\t\t\tcontinue;\n-\t\t} else if (atom_type == ATOM_ELSE) {\n+\t\t\tbreak;\n+\t\t}\n+\t\tcase ATOM_ELSE: {\n \t\t\tv->handler = else_atom_handler;\n \t\t\tv->s = ref_filter_slopbuf;\n-\t\t\tcontinue;\n-\t\t} else if (atom_type == ATOM_REST) {\n+\t\t\tbreak;\n+\t\t}\n+\t\tcase ATOM_REST: {\n \t\t\tif (ref->rest)\n \t\t\t\tv->s = xstrdup(ref->rest);\n \t\t\telse\n \t\t\t\tv->s = ref_filter_slopbuf;\n-\t\t\tcontinue;\n-\t\t} else\n-\t\t\tcontinue;\n-\n-\t\tif (!deref)\n-\t\t\tv->s = xstrdup(refname);\n-\t\telse\n-\t\t\tv->s = xstrfmt_len(&v->s_size, \"%s^{}\", refname);\n-\t\tfree((char *)refname);\n+\t\t\tbreak;\n+\t\t}\n+\t\tdefault:\n+\t\t\tbreak;\n+\t\t}\n \t}\n \n \tfor (i = 0; i < used_atom_cnt; i++) {\n-- \ngitgitgadget\n"}]}