{"thread":{"id":"40575","subject":"git tag --contains now takes a long time","startedAt":"2015-10-16T22:07:39Z","lastAt":"2015-10-18T10:04:42Z","messageCount":8,"participants":["Jerry Snitselaar","Junio C Hamano","Karthik Nayak","Matthieu Moy"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"271840","messageId":"20151016220739.GF17700@cantor.redhat.com","threadId":"40575","inReplyTo":null,"subject":"git tag --contains now takes a long time","fromName":"Jerry Snitselaar","fromEmail":"jsnitsel@redhat.com","sentAt":"2015-10-16T22:07:39Z","receivedAt":"2015-10-16T22:07:39Z","isPatch":false,"sender":{"key":"jsnitsel@redhat.com","avatar":null},"body":"Is this known and being looked into? I've seen a jump from 12 seconds\nto over 9 minutes with running git tag --contains on my kernel repo.\n\n\nsnits ~/dev/linux> git --version\ngit version 2.6.1.145.gb27dacc\n\nsnits ~/dev/linux> time git tag --contains 825fcfc\nnext-20151012\nnext-20151013\nv4.3-rc5\n\nreal\t9m4.765s\nuser\t8m56.157s\nsys\t0m2.450s\n\n\n\nsnits ~/dev/linux> git --version\ngit version 2.5.0.275.gac4cc86\n\nsnits ~/dev/linux> time git tag --contains 825fcfc\nnext-20151012\nnext-20151013\nv4.3-rc5\n\nreal\t0m12.842s\nuser\t0m11.536s\nsys\t0m1.098s\n\n\n\nb7cc53e92c806b73e14b03f60c17b7c29e52b4a4 is the first bad commit\ncommit b7cc53e92c806b73e14b03f60c17b7c29e52b4a4\nAuthor: Karthik Nayak \nDate: Fri Sep 11 20:36:16 2015 +0530\n\n tag.c: use 'ref-filter' APIs\n\n Make 'tag.c' use 'ref-filter' APIs for iterating through refs, sorting\n and printing of refs. This removes most of the code used in 'tag.c'\n replacing it with calls to the 'ref-filter' library.\n\n Make 'tag.c' use the 'filter_refs()' function provided by 'ref-filter'\n to filter out tags based on the options set.\n\n For printing tags we use 'show_ref_array_item()' function provided by\n 'ref-filter'.\n\n We improve the sorting option provided by 'tag.c' by using the sorting\n options provided by 'ref-filter'. This causes the test 'invalid sort\n parameter on command line' in t7004 to fail, as 'ref-filter' throws an\n error for all sorting fields which are incorrect. The test is changed\n to reflect the same.\n\n Modify documentation for the same.\n\n Mentored-by: Christian Couder \n Mentored-by: Matthieu Moy \n Signed-off-by: Karthik Nayak \n Signed-off-by: Junio C Hamano \n"},{"id":"271843","messageId":"xmqqd1wecuea.fsf@gitster.mtv.corp.google.com","threadId":"40575","inReplyTo":"20151016220739.GF17700@cantor.redhat.com","subject":"Re: git tag --contains now takes a long time","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-10-16T22:37:17Z","receivedAt":"2015-10-16T22:37:17Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jerry Snitselaar writes:\n\n> Is this known and being looked into? I've seen a jump from 12 seconds\n> to over 9 minutes with running git tag --contains on my kernel repo.\n\nThanks for a report. Yes, it seems that there is a recent\nregression on the 'master' branch, not yet in any released version,\nand we can observe with a much smaller repository X-<. E.g.\n\n git tag --contains v2.5.0~12\n\nin git itself seems to have got 10 times slower.\n\n\n> snits ~/dev/linux> git --version\n> git version 2.6.1.145.gb27dacc\n>\n> snits ~/dev/linux> time git tag --contains 825fcfc\n> next-20151012\n> next-20151013\n> v4.3-rc5\n>\n> real\t9m4.765s\n> user\t8m56.157s\n> sys\t0m2.450s\n>\n>\n>\n> snits ~/dev/linux> git --version\n> git version 2.5.0.275.gac4cc86\n>\n> snits ~/dev/linux> time git tag --contains 825fcfc\n> next-20151012\n> next-20151013\n> v4.3-rc5\n>\n> real\t0m12.842s\n> user\t0m11.536s\n> sys\t0m1.098s\n>\n>\n>\n> b7cc53e92c806b73e14b03f60c17b7c29e52b4a4 is the first bad commit\n> commit b7cc53e92c806b73e14b03f60c17b7c29e52b4a4\n> Author: Karthik Nayak \n> Date: Fri Sep 11 20:36:16 2015 +0530\n>\n> tag.c: use 'ref-filter' APIs\n>\n> Make 'tag.c' use 'ref-filter' APIs for iterating through refs, sorting\n> and printing of refs. This removes most of the code used in 'tag.c'\n> replacing it with calls to the 'ref-filter' library.\n>\n> Make 'tag.c' use the 'filter_refs()' function provided by 'ref-filter'\n> to filter out tags based on the options set.\n>\n> For printing tags we use 'show_ref_array_item()' function provided by\n> 'ref-filter'.\n>\n> We improve the sorting option provided by 'tag.c' by using the sorting\n> options provided by 'ref-filter'. This causes the test 'invalid sort\n> parameter on command line' in t7004 to fail, as 'ref-filter' throws an\n> error for all sorting fields which are incorrect. The test is changed\n> to reflect the same.\n>\n> Modify documentation for the same.\n>\n> Mentored-by: Christian Couder \n> Mentored-by: Matthieu Moy \n> Signed-off-by: Karthik Nayak \n> Signed-off-by: Junio C Hamano \n"},{"id":"271846","messageId":"CAOLa=ZTDd3MSmqXArtNz8i5n=uj2NEB6UPk2jSnEhUsAqbr7nQ@mail.gmail.com","threadId":"40575","inReplyTo":"20151016220739.GF17700@cantor.redhat.com","subject":"Re: git tag --contains now takes a long time","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-10-17T06:44:58Z","receivedAt":"2015-10-17T06:44:58Z","isPatch":false,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Sat, Oct 17, 2015 at 3:37 AM, Jerry Snitselaar wrote:\n> Is this known and being looked into? I've seen a jump from 12 seconds\n> to over 9 minutes with running git tag --contains on my kernel repo.\n>\n\nThis wasn't known, thanks for bringing it up.\n\n>\n> snits ~/dev/linux> git --version\n> git version 2.6.1.145.gb27dacc\n>\n> snits ~/dev/linux> time git tag --contains 825fcfc\n> next-20151012\n> next-20151013\n> v4.3-rc5\n>\n> real 9m4.765s\n> user 8m56.157s\n> sys 0m2.450s\n>\n>\n>\n> snits ~/dev/linux> git --version\n> git version 2.5.0.275.gac4cc86\n>\n> snits ~/dev/linux> time git tag --contains 825fcfc\n> next-20151012\n> next-20151013\n> v4.3-rc5\n>\n> real 0m12.842s\n> user 0m11.536s\n> sys 0m1.098s\n>\n>\n> b7cc53e92c806b73e14b03f60c17b7c29e52b4a4 is the first bad commit\n> commit b7cc53e92c806b73e14b03f60c17b7c29e52b4a4\n> Author: Karthik Nayak \n> Date: Fri Sep 11 20:36:16 2015 +0530\n>\n> tag.c: use 'ref-filter' APIs\n>\n> Make 'tag.c' use 'ref-filter' APIs for iterating through refs, sorting\n> and printing of refs. This removes most of the code used in 'tag.c'\n> replacing it with calls to the 'ref-filter' library.\n>\n> Make 'tag.c' use the 'filter_refs()' function provided by 'ref-filter'\n> to filter out tags based on the options set.\n>\n> For printing tags we use 'show_ref_array_item()' function provided by\n> 'ref-filter'.\n>\n> We improve the sorting option provided by 'tag.c' by using the sorting\n> options provided by 'ref-filter'. This causes the test 'invalid sort\n> parameter on command line' in t7004 to fail, as 'ref-filter' throws an\n> error for all sorting fields which are incorrect. The test is changed\n> to reflect the same.\n>\n> Modify documentation for the same.\n>\n> Mentored-by: Christian Couder \n> Mentored-by: Matthieu Moy \n> Signed-off-by: Karthik Nayak \n> Signed-off-by: Junio C Hamano \n\nAs Junio mentioned I did notice ~13x slower speed.\n\n $ git version\n\n[12:02:08]\ngit version 2.5.0.275.gac4cc86\n\n$ time git tag --contains HEAD~300\n [12:06:03]\nnext-20150924\nnext-20151016\nv4.3-rc1\nv4.3-rc2\nv4.3-rc3\nv4.3-rc4\nv4.3-rc5\ngit tag --contains HEAD~300 2.89s user 0.14s system 99% cpu 3.031 total\n\nAfter applying b7cc53e92c806b73e14b03f60c17b7c29e52b4a4\n\n $ git version\n\n[12:07:33]\ngit version 2.5.0.276.gb7cc53e\n\n $ time git tag --contains HEAD~300\n [12:07:35]\nnext-20150924\nnext-20151016\nv4.3-rc1\nv4.3-rc2\nv4.3-rc3\nv4.3-rc4\nv4.3-rc5\n../Git/git tag --contains HEAD~300 38.30s user 0.19s system 99% cpu\n38.519 total\n\nSo I did poke around a little. I think I missed this out on the\noriginal commit (b7cc53e92c806b73e14b03f60c17b7c29e52b4a4).\n\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex 977a18c..2c5a9f1 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -49,6 +49,7 @@ static int list_tags(struct ref_filter *filter,\nstruct ref_sorting *sorting)\n format = \"%(refname:short)\";\n\n verify_ref_format(format);\n+ filter->with_commit_tag_algo = 1;\n filter_refs(&array, filter, FILTER_REFS_TAGS);\n ref_array_sort(sorting, &array);\n\nAfter applying this and running the test again, we get:\n\n $ git version\n\n[12:09:13]\ngit version 2.5.0.276.gb7cc53e.dirty\n\n $ time git tag --contains HEAD~300\n [12:12:00]\nnext-20150924\nnext-20151016\nv4.3-rc1\nv4.3-rc2\nv4.3-rc3\nv4.3-rc4\nv4.3-rc5\n../Git/git tag --contains HEAD~300 2.85s user 0.18s system 99% cpu 3.031 total\n\n\nCould you Squash that in, Junio?\n\n-- \nRegards,\nKarthik Nayak\n"},{"id":"271847","messageId":"20151017095120.GB4496@cantor.redhat.com","threadId":"40575","inReplyTo":"CAOLa=ZTDd3MSmqXArtNz8i5n=uj2NEB6UPk2jSnEhUsAqbr7nQ@mail.gmail.com","subject":"Re: git tag --contains now takes a long time","fromName":"Jerry Snitselaar","fromEmail":"jsnitsel@redhat.com","sentAt":"2015-10-17T09:51:20Z","receivedAt":"2015-10-17T09:51:20Z","isPatch":false,"sender":{"key":"jsnitsel@redhat.com","avatar":null},"body":"On Sat Oct 17 15, Karthik Nayak wrote:\n>On Sat, Oct 17, 2015 at 3:37 AM, Jerry Snitselaar wrote:\n>> Is this known and being looked into? I've seen a jump from 12 seconds\n>> to over 9 minutes with running git tag --contains on my kernel repo.\n>>\n>\n>This wasn't known, thanks for bringing it up.\n>\n>>\n>> snits ~/dev/linux> git --version\n>> git version 2.6.1.145.gb27dacc\n>>\n>> snits ~/dev/linux> time git tag --contains 825fcfc\n>> next-20151012\n>> next-20151013\n>> v4.3-rc5\n>>\n>> real 9m4.765s\n>> user 8m56.157s\n>> sys 0m2.450s\n>>\n>>\n>>\n>> snits ~/dev/linux> git --version\n>> git version 2.5.0.275.gac4cc86\n>>\n>> snits ~/dev/linux> time git tag --contains 825fcfc\n>> next-20151012\n>> next-20151013\n>> v4.3-rc5\n>>\n>> real 0m12.842s\n>> user 0m11.536s\n>> sys 0m1.098s\n>>\n>>\n>> b7cc53e92c806b73e14b03f60c17b7c29e52b4a4 is the first bad commit\n>> commit b7cc53e92c806b73e14b03f60c17b7c29e52b4a4\n>> Author: Karthik Nayak \n>> Date: Fri Sep 11 20:36:16 2015 +0530\n>>\n>> tag.c: use 'ref-filter' APIs\n>>\n>> Make 'tag.c' use 'ref-filter' APIs for iterating through refs, sorting\n>> and printing of refs. This removes most of the code used in 'tag.c'\n>> replacing it with calls to the 'ref-filter' library.\n>>\n>> Make 'tag.c' use the 'filter_refs()' function provided by 'ref-filter'\n>> to filter out tags based on the options set.\n>>\n>> For printing tags we use 'show_ref_array_item()' function provided by\n>> 'ref-filter'.\n>>\n>> We improve the sorting option provided by 'tag.c' by using the sorting\n>> options provided by 'ref-filter'. This causes the test 'invalid sort\n>> parameter on command line' in t7004 to fail, as 'ref-filter' throws an\n>> error for all sorting fields which are incorrect. The test is changed\n>> to reflect the same.\n>>\n>> Modify documentation for the same.\n>>\n>> Mentored-by: Christian Couder \n>> Mentored-by: Matthieu Moy \n>> Signed-off-by: Karthik Nayak \n>> Signed-off-by: Junio C Hamano \n>\n>As Junio mentioned I did notice ~13x slower speed.\n>\n> $ git version\n>\n>[12:02:08]\n>git version 2.5.0.275.gac4cc86\n>\n>$ time git tag --contains HEAD~300\n> [12:06:03]\n>next-20150924\n>next-20151016\n>v4.3-rc1\n>v4.3-rc2\n>v4.3-rc3\n>v4.3-rc4\n>v4.3-rc5\n>git tag --contains HEAD~300 2.89s user 0.14s system 99% cpu 3.031 total\n>\n>After applying b7cc53e92c806b73e14b03f60c17b7c29e52b4a4\n>\n> $ git version\n>\n>[12:07:33]\n>git version 2.5.0.276.gb7cc53e\n>\n> $ time git tag --contains HEAD~300\n> [12:07:35]\n>next-20150924\n>next-20151016\n>v4.3-rc1\n>v4.3-rc2\n>v4.3-rc3\n>v4.3-rc4\n>v4.3-rc5\n>../Git/git tag --contains HEAD~300 38.30s user 0.19s system 99% cpu\n>38.519 total\n>\n>So I did poke around a little. I think I missed this out on the\n>original commit (b7cc53e92c806b73e14b03f60c17b7c29e52b4a4).\n>\n>diff --git a/builtin/tag.c b/builtin/tag.c\n>index 977a18c..2c5a9f1 100644\n>--- a/builtin/tag.c\n>+++ b/builtin/tag.c\n>@@ -49,6 +49,7 @@ static int list_tags(struct ref_filter *filter,\n>struct ref_sorting *sorting)\n> format = \"%(refname:short)\";\n>\n> verify_ref_format(format);\n>+ filter->with_commit_tag_algo = 1;\n> filter_refs(&array, filter, FILTER_REFS_TAGS);\n> ref_array_sort(sorting, &array);\n>\n>After applying this and running the test again, we get:\n>\n> $ git version\n>\n>[12:09:13]\n>git version 2.5.0.276.gb7cc53e.dirty\n>\n> $ time git tag --contains HEAD~300\n> [12:12:00]\n>next-20150924\n>next-20151016\n>v4.3-rc1\n>v4.3-rc2\n>v4.3-rc3\n>v4.3-rc4\n>v4.3-rc5\n>../Git/git tag --contains HEAD~300 2.85s user 0.18s system 99% cpu 3.031 total\n>\n>\n>Could you Squash that in, Junio?\n>\n>-- \n>Regards,\n>Karthik Nayak\n\nsnits ~/dev/linux> time git tag --contains 825fcfc\nnext-20151012\nnext-20151013\nv4.3-rc5\n\nreal\t0m16.998s\nuser\t0m12.539s\nsys\t0m1.572s\n\n\nThat worked for me.\n\n\nTested-by: Jerry Snitselaar \n"},{"id":"271856","messageId":"vpqmvvhpjv2.fsf@grenoble-inp.fr","threadId":"40575","inReplyTo":"CAOLa=ZTDd3MSmqXArtNz8i5n=uj2NEB6UPk2jSnEhUsAqbr7nQ@mail.gmail.com","subject":"Re: git tag --contains now takes a long time","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-10-17T15:58:41Z","receivedAt":"2015-10-17T15:58:41Z","isPatch":false,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Karthik Nayak writes:\n\n> Could you Squash that in, Junio?\n\nThe guilty commit is in master, so you'll need a new commit to fix the\nold one. Can you send a patch with a proper commit message (referencing\nb7cc53e92c806b73e14b03f60c17b7c29e52b4a4)?\n\nWe should both have catched this earlier (by review and/or benchmark).\nSorry we didn't, and thanks to Jerry for the report.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"271857","messageId":"CAOLa=ZSjNjey3zWozFkyFw-X-BPtsVNpQKqG5hO9-7qJOBR_KA@mail.gmail.com","threadId":"40575","inReplyTo":"vpqmvvhpjv2.fsf@grenoble-inp.fr","subject":"Re: git tag --contains now takes a long time","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-10-17T18:10:23Z","receivedAt":"2015-10-17T18:10:23Z","isPatch":false,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Sat, Oct 17, 2015 at 9:28 PM, Matthieu Moy\n wrote:\n> Karthik Nayak writes:\n>\n>> Could you Squash that in, Junio?\n>\n> The guilty commit is in master, so you'll need a new commit to fix the\n> old one. Can you send a patch with a proper commit message (referencing\n> b7cc53e92c806b73e14b03f60c17b7c29e52b4a4)?\n>\n> We should both have catched this earlier (by review and/or benchmark).\n> Sorry we didn't, and thanks to Jerry for the report.\n>\n\nYea, I'm surprised we didn't notice. Either ways, I'll reply with a\nfix commit :)\n\n-- \nRegards,\nKarthik Nayak\n"},{"id":"271861","messageId":"xmqq1tctchgz.fsf@gitster.mtv.corp.google.com","threadId":"40575","inReplyTo":"CAOLa=ZTDd3MSmqXArtNz8i5n=uj2NEB6UPk2jSnEhUsAqbr7nQ@mail.gmail.com","subject":"Re: git tag --contains now takes a long time","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-10-17T21:28:44Z","receivedAt":"2015-10-17T21:28:44Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Karthik Nayak writes:\n\n> So I did poke around a little. I think I missed this out on the\n> original commit (b7cc53e92c806b73e14b03f60c17b7c29e52b4a4).\n>\n> diff --git a/builtin/tag.c b/builtin/tag.c\n> index 977a18c..2c5a9f1 100644\n> --- a/builtin/tag.c\n> +++ b/builtin/tag.c\n> @@ -49,6 +49,7 @@ static int list_tags(struct ref_filter *filter,\n> struct ref_sorting *sorting)\n> format = \"%(refname:short)\";\n>\n> verify_ref_format(format);\n> + filter->with_commit_tag_algo = 1;\n> filter_refs(&array, filter, FILTER_REFS_TAGS);\n> ref_array_sort(sorting, &array);\n> ...\n>\n> Could you Squash that in, Junio?\n\nDo we have two implementations that are supposed to compute the same\nthing, and with the bit set to 1, the faster of these two is used?\nIs there a reason somebody may want to use the slower one? What\ndifference other than performance does the choice of this bit makes,\nand why?\n\nI think the answers to the above questions deserve to be in the log\nmessage (no, I do not think I can \"squash it in\", rewriting the\ncommit that has already been merged to 'next' and 'master').\n\nThanks.\n"},{"id":"271867","messageId":"CAOLa=ZQNL1YUFzWYCiYhRf4ESmeCTtZEXzbbf39d7DqSkbj66g@mail.gmail.com","threadId":"40575","inReplyTo":"xmqq1tctchgz.fsf@gitster.mtv.corp.google.com","subject":"Re: git tag --contains now takes a long time","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-10-18T10:04:42Z","receivedAt":"2015-10-18T10:04:42Z","isPatch":false,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Sun, Oct 18, 2015 at 2:58 AM, Junio C Hamano wrote:\n> Karthik Nayak writes:\n>\n>> So I did poke around a little. I think I missed this out on the\n>> original commit (b7cc53e92c806b73e14b03f60c17b7c29e52b4a4).\n>>\n>> diff --git a/builtin/tag.c b/builtin/tag.c\n>> index 977a18c..2c5a9f1 100644\n>> --- a/builtin/tag.c\n>> +++ b/builtin/tag.c\n>> @@ -49,6 +49,7 @@ static int list_tags(struct ref_filter *filter,\n>> struct ref_sorting *sorting)\n>> format = \"%(refname:short)\";\n>>\n>> verify_ref_format(format);\n>> + filter->with_commit_tag_algo = 1;\n>> filter_refs(&array, filter, FILTER_REFS_TAGS);\n>> ref_array_sort(sorting, &array);\n>> ...\n>>\n>> Could you Squash that in, Junio?\n>\n> Do we have two implementations that are supposed to compute the same\n> thing, and with the bit set to 1, the faster of these two is used?\n> Is there a reason somebody may want to use the slower one? What\n> difference other than performance does the choice of this bit makes,\n> and why?\n>\n> I think the answers to the above questions deserve to be in the log\n> message (no, I do not think I can \"squash it in\", rewriting the\n> commit that has already been merged to 'next' and 'master').\n>\n> Thanks.\n\nI'll resend the patch then with the changed commit message.\n\nThanks.\n\n-- \nRegards,\nKarthik Nayak\n"}]}