{"thread":{"id":"56285","subject":"[GSoC] Git Blog 13","startedAt":"2021-08-15T07:08:21Z","lastAt":"2021-08-16T09:13:35Z","messageCount":3,"participants":["ZheNing Hu","Christian Couder"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"432758","messageId":"CAOLTT8Q6f-B05bQ+XCDuGGk5KTS1o3QatGbeJ-FQiqFCYGW7Og@mail.gmail.com","threadId":"56285","inReplyTo":null,"subject":"[GSoC] Git Blog 13","fromName":"ZheNing Hu","fromEmail":"adlternative@gmail.com","sentAt":"2021-08-15T07:08:04Z","receivedAt":"2021-08-15T07:08:21Z","isPatch":false,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"My thirteenth week blog finished:\nThe web version is here:\nhttps://adlternative.github.io/GSOC-Git-Blog-13/\n\n### Project Progress\n\nThis week I continue to try to optimize the performance of `git\ncat-file --batch`.\n\nYou can see them here:\n\n```bash\ngit fetch git@github.com:adlternative/git.git cat-file-reuse-ref-filter-logic\ngit rev-list 2c6ce95c82..8591897fbc\n```\n\nOr here:\n\n[[PATCH 00/27] [GSOC] [RFC] cat-file: reuse ref-filter\nlogic](https://lore.kernel.org/git/pull.1016.git.1628842990.gitgitgadget@gmail.com/)\n\nSeveral of these commits are critical:\n\n* 31acac9fde `[GSOC] ref-filter: remove second parsing in\nformat_ref_array_item`:\nIt use a `parsed_atom_list` to save the parsed state after\nwe calling verify_ref_format(), so we can reduce second\nparsing in format_ref_array_item(), which brings 1.9%\nperformance improvement.\n* 4602b62a92 `[GSOC] ref-filter: reuse finnal buffer if no\nstack need`：It can reduce some unnecessary copies,\nwhich bring 2% performance improvement.\n\nThis time I made sure that there was not too much noise,\nto ensure the stability of this performance test：\n\n#### Test Examples：\n* upstream/master: `5d213e46bb (tag: v2.33.0-rc2, upstream/master) Git 2.33-rc2`\n* 898e36a92b (before performance optimization): `898e36a92b [GSOC]\ncat-file: re-implement --textconv, --filters options`\n* this tree (after performance optimization): `8591897fbc (HEAD ->\ncat-file-reuse-ref-filter-logic) [GSOC] ref-filter: add\nneed_get_object_info flag to struct expand_data`\n\n\n#### Test Results：\n```bash\n$ GIT_PERF_REPEAT_COUNT=50 GIT_PERF_MAKE_OPTS=-j8 ./run\nupstream/master . ./p1006-cat-file.sh\n\nTest upstream/master this tree\n------------------------------------------------------------------------------------\n1006.2: cat-file --batch-check 0.08(0.06+0.01)\n0.08(0.07+0.01) +0.0%\n1006.3: cat-file --batch-check with atoms 0.06(0.05+0.00)\n0.07(0.06+0.01) +16.7%\n1006.4: cat-file --batch 0.48(0.45+0.03)\n0.50(0.46+0.03) +4.2%\n1006.5: cat-file --batch with atoms 0.47(0.43+0.03)\n0.49(0.46+0.02) +4.3%\n\n$ GIT_PERF_REPEAT_COUNT=50 GIT_PERF_MAKE_OPTS=-j8 ./run\nupstream/master 898e36a92b ./p1006-cat-file.sh\n\nTest upstream/master 898e36a92b\n------------------------------------------------------------------------------------\n1006.2: cat-file --batch-check 0.08(0.07+0.00)\n0.09(0.09+0.00) +12.5%\n1006.3: cat-file --batch-check with atoms 0.06(0.04+0.01)\n0.07(0.05+0.02) +16.7%\n1006.4: cat-file --batch 0.48(0.44+0.03)\n0.60(0.58+0.02) +25.0%\n1006.5: cat-file --batch with atoms 0.47(0.44+0.02)\n0.58(0.56+0.02) +23.4%\n```\n\nThe performance of `git cat-file --batch-check` is very close\nto `upstream/master`!\n\nThe performance difference of `git cat-file --batch` has also\nchanged from 25.0% to 4.2%!\n\nThis result is far better than my expectations, I have reason to\nbelieve that the performance of `git cat-file --batch` can be\nimproved again!\n\nGood job!\n\nI think the key to continuing to optimize is still to reduce unnecessary copies.\n\n### Additional advice\n\nDuring the optimization process this week, I found that I might want\nto use a `strbuf_move()` function, although I did not adopt it in my work\n(because it did not allow my work to be greatly optimized), but I think it might\nbe useful in some situations: We don’t want to copy the data of strbuf,\nbut just want to move its buf pointer:\n\n```c\nvoid strbuf_move(struct strbuf *sb, struct strbuf *sb2)\n{\nstrbuf_release(sb);\n*sb = *sb2;\nstrbuf_init(sb2, 0);\n}\n```\n\nYes, it's like `std::move()` in c++.\n[std::move](https://en.cppreference.com/w/cpp/utility/move)\nMaybe we can use strbuf_move() in some scenarios.\n\nThanks, Git!\n--\nZheNing Hu\n"},{"id":"432759","messageId":"CAP8UFD3FpnmOi50iUuFa9qMzi=PVP_Kcod6T+8BzJNq5X-cmgw@mail.gmail.com","threadId":"56285","inReplyTo":"CAOLTT8Q6f-B05bQ+XCDuGGk5KTS1o3QatGbeJ-FQiqFCYGW7Og@mail.gmail.com","subject":"Re: [GSoC] Git Blog 13","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2021-08-15T10:28:12Z","receivedAt":"2021-08-15T10:28:26Z","isPatch":false,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Sun, Aug 15, 2021 at 9:08 AM ZheNing Hu wrote:\n>\n> My thirteenth week blog finished:\n> The web version is here:\n> https://adlternative.github.io/GSOC-Git-Blog-13/\n\nThanks!\n\n> ### Project Progress\n>\n> This week I continue to try to optimize the performance of `git\n> cat-file --batch`.\n>\n> You can see them here:\n\n\n> Test upstream/master this tree\n> ------------------------------------------------------------------------------------\n> 1006.2: cat-file --batch-check 0.08(0.06+0.01)\n> 0.08(0.07+0.01) +0.0%\n> 1006.3: cat-file --batch-check with atoms 0.06(0.05+0.00)\n> 0.07(0.06+0.01) +16.7%\n> 1006.4: cat-file --batch 0.48(0.45+0.03)\n> 0.50(0.46+0.03) +4.2%\n> 1006.5: cat-file --batch with atoms 0.47(0.43+0.03)\n> 0.49(0.46+0.02) +4.3%\n\n\n> The performance of `git cat-file --batch-check` is very close\n> to `upstream/master`!\n>\n> The performance difference of `git cat-file --batch` has also\n> changed from 25.0% to 4.2%!\n>\n> This result is far better than my expectations, I have reason to\n> believe that the performance of `git cat-file --batch` can be\n> improved again!\n>\n> Good job!\n\nYeah, great!\n\n> I think the key to continuing to optimize is still to reduce unnecessary copies.\n\nDo you have data or hints about this?\n\nHave you looked at why there is still +16.7% in the cat-file\n--batch-check with atoms case? Maybe solving this would improve things\nin the other cases too.\n\n> ### Additional advice\n>\n> During the optimization process this week, I found that I might want\n> to use a `strbuf_move()` function, although I did not adopt it in my work\n> (because it did not allow my work to be greatly optimized), but I think it might\n> be useful in some situations: We don’t want to copy the data of strbuf,\n> but just want to move its buf pointer:\n\nYeah, if we are sure that it improves performance, that might be worth it.\n\nAbout your patch series\n(https://lore.kernel.org/git/pull.1016.git.1628842990.gitgitgadget@gmail.com/),\nI wonder if it might be possible and better to have some performance\nimprovements before the commit that uses the ref-filter code\n(\"cat-file: reuse ref-filter logic\"). If you are going this way, it\nmight make sense to add a few performance tests, if some don't already\nexist, to show the effect of these performance improvements on the\nref-filter code when it's used by other commands like for-each-ref.\n\nThis could show that these performance improvements are worth doing\neven if we didn't want to reuse the ref-filter code in cat-file. And\nthen perhaps these performance improvements could be merged as part of\none or more small patch series, before the patch series that reuses\nthe ref-filter code in cat-file.\n\nBest,\nChristian.\n"},{"id":"432790","messageId":"CAOLTT8Rfai0bHJaUYHLU05guEApOacOhurjTZrP+VEw0YwenPA@mail.gmail.com","threadId":"56285","inReplyTo":"CAP8UFD3FpnmOi50iUuFa9qMzi=PVP_Kcod6T+8BzJNq5X-cmgw@mail.gmail.com","subject":"Re: [GSoC] Git Blog 13","fromName":"ZheNing Hu","fromEmail":"adlternative@gmail.com","sentAt":"2021-08-16T09:13:19Z","receivedAt":"2021-08-16T09:13:35Z","isPatch":false,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"Christian Couder 于2021年8月15日周日 下午6:28写道：\n>\n> > I think the key to continuing to optimize is still to reduce unnecessary copies.\n>\n> Do you have data or hints about this?\n>\n\nHere is the result of perf diff with git cat-file --batch-check\n--batch-all-objects\nbetween upstream/master and cat-file-reuse-ref-filter-logic:\n\n# Event 'cycles:u'\n#\n# Baseline Delta Abs Shared Object Symbol\n# ........ ......... ................ ..................................\n#\n 18.08% -2.11% libz.so.1.2.11 [.] 0x0000000000008dde\n +2.09% libc-2.33.so [.] malloc_consolidate\n 21.65% -1.70% git [.] unpack_object_header_buffer\n 5.62% -1.57% git [.] use_pack\n 0.59% +1.49% libc-2.33.so [.] _int_free\n +1.40% git [.] get_object\n 13.62% -1.39% libz.so.1.2.11 [.] inflate\n 1.54% -0.98% libc-2.33.so [.] __strchrnul_avx2\n +0.90% git [.] format_ref_array_item\n +0.82% git [.] populate_value\n 0.77% +0.74% libc-2.33.so [.] __strlen_avx2\n 0.37% +0.68% libc-2.33.so [.] cfree@GLIBC_2.2.5\n +0.56% git [.] append_literal.isra.0\n 0.32% +0.53% libc-2.33.so [.] malloc\n 1.09% -0.48% git [.] get_size_from_delta\n 0.77% -0.46% git [.] oid_array_for_each_unique\n 2.48% -0.45% git [.] get_delta_base\n 0.20% +0.41% libc-2.33.so [.] _IO_fwrite\n +0.41% git [.] batch_object_write.constprop.0\n +0.40% git [.] quote_formatting\n +0.39% git [.] get_ref_atom_value\n 0.31% +0.38% git [.] strbuf_add\n +0.36% libc-2.33.so [.] __libc_calloc\n 0.41% +0.35% [unknown] [k] 0xffffffff84c00158\n 0.58% -0.33% libc-2.33.so [.] unlink_chunk.constprop.0\n 1.04% -0.31% libc-2.33.so [.] __GI___qsort_r\n +0.30% git [.] 0x000000000001cd80\n +0.30% git [.] append_atom\n 3.23% +0.30% libc-2.33.so [.] __memmove_avx_unaligned_erms\n\nmalloc_consolidate(), _int_free(), malloc(), __libc_calloc(), they\ntake up a larger\nproportion of time (through gprof, we can also see that the number of\ncalls is also\nincreasing), it shows that our memory allocation is worth optimizing.\n\n__memmove_avx_unaligned_erms() take up a larger proportion of time,\nit shows that we have some unnecessary copies.\n\nNote that 4602b62 ([GSOC] ref-filter: reuse finnal buffer if no stack\nneed) is trying\nto alleviate this problem, and the results show that it does have a 2%\noptimization.\n\nSo today I added two new commits to verify my guess: We need to reduce copying.\nSee: https://github.com/adlternative/git/commits/cat-file-reuse-ref-filter-logic-temp-5\n\n\nTest upstream/master this\ntree\n------------------------------------------------------------------------------------\n1006.2: cat-file --batch-check 0.08(0.06+0.01)\n0.08(0.06+0.01) +0.0%\n1006.3: cat-file --batch-check with atoms 0.06(0.04+0.01)\n0.07(0.06+0.01) +16.7%\n1006.4: cat-file --batch 0.49(0.47+0.02)\n0.47(0.46+0.00) -4.1%\n1006.5: cat-file --batch with atoms 0.47(0.45+0.01)\n0.46(0.46+0.00) -2.1%\n\nThis is good news. The performance of our git cat-file --batch is\nbetter than before!\n\n> Have you looked at why there is still +16.7% in the cat-file\n> --batch-check with atoms case? Maybe solving this would improve things\n> in the other cases too.\n>\n\nI am continuing to think about it, I guess it is still a\nmalloc/free/memmove problem.\nThe execution time of git cat-file --batch-check is relatively short,\nand the disadvantages\nof using ref-filter logic will be more prominent.\n\n> > ### Additional advice\n> >\n> > During the optimization process this week, I found that I might want\n> > to use a `strbuf_move()` function, although I did not adopt it in my work\n> > (because it did not allow my work to be greatly optimized), but I think it might\n> > be useful in some situations: We don’t want to copy the data of strbuf,\n> > but just want to move its buf pointer:\n>\n> Yeah, if we are sure that it improves performance, that might be worth it.\n>\n> About your patch series\n> (https://lore.kernel.org/git/pull.1016.git.1628842990.gitgitgadget@gmail.com/),\n> I wonder if it might be possible and better to have some performance\n> improvements before the commit that uses the ref-filter code\n> (\"cat-file: reuse ref-filter logic\"). If you are going this way, it\n> might make sense to add a few performance tests, if some don't already\n> exist, to show the effect of these performance improvements on the\n> ref-filter code when it's used by other commands like for-each-ref.\n>\n> This could show that these performance improvements are worth doing\n> even if we didn't want to reuse the ref-filter code in cat-file. And\n> then perhaps these performance improvements could be merged as part of\n> one or more small patch series, before the patch series that reuses\n> the ref-filter code in cat-file.\n>\n\nAh, this is estimated to take a lot of effort. I feel that it will\ntake a long time\nfor these optimization patches to be merged into the master.\n\nI tried to add a performance test for for-each-ref, but the effect is very\ninsignificant, because its execution time is too short.\n\n> Best,\n> Christian.\n\nThanks.\n--\nZheNing Hu\n"}]}