{"thread":{"id":"54188","subject":"[PATCH] submodule: suppress checking for file name ambiguity for object ids","startedAt":"2020-09-04T14:53:23Z","lastAt":"2020-09-06T21:59:49Z","messageCount":7,"participants":["Orgad Shaneh via GitGitGadget","Orgad Shaneh","Ramsay Jones","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"405043","messageId":"pull.725.git.1599231196975.gitgitgadget@gmail.com","threadId":"54188","inReplyTo":null,"subject":"[PATCH] submodule: suppress checking for file name ambiguity for object ids","fromName":"Orgad Shaneh via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-09-04T14:53:16Z","receivedAt":"2020-09-04T14:53:23Z","isPatch":true,"sender":{"key":"orgads@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1246544?v=4"},"body":"From: Orgad Shaneh <orgads@gmail.com>\n\nThe argv argument of collect_changed_submodules() contains obly object ids\n(submodule references).\n\nNotify setup_revisions() that the input is not filenames by passing\nassume_dashdash, so it can avoid redundant stat for each ref.\n\nA better improvement would be to pass oid_array instead of stringified argv,\nbut that will require a larger change, which can be done later.\n\nSigned-off-by: Orgad Shaneh <orgads@gmail.com>\n---\n    submodule: suppress checking for file name ambiguity for object ids\n    \n    The argv argument of collect_changed_submodules() contains obly object\n    ids (submodule references).\n    \n    Notify setup_revisions() that the input is not filenames by passing\n    assume_dashdash, so it can avoid redundant stat for each ref.\n    \n    A better improvement would be to pass oid_array instead of stringified\n    argv, but that will require a larger change, which can be done later.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-725%2Forgads%2Fsubmodule-not-filename-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-725/orgads/submodule-not-filename-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/725\n\n submodule.c | 5 ++++-\n 1 file changed, 4 insertions(+), 1 deletion(-)\n\ndiff --git a/submodule.c b/submodule.c\nindex 3cbcf40dfc..9b5bfb12a3 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -840,9 +840,12 @@ static void collect_changed_submodules(struct repository *r,\n {\n \tstruct rev_info rev;\n \tconst struct commit *commit;\n+\tstruct setup_revision_opt s_r_opt = {\n+\t\t.assume_dashdash = 1,\n+\t};\n \n \trepo_init_revisions(r, &rev, NULL);\n-\tsetup_revisions(argv->nr, argv->v, &rev, NULL);\n+\tsetup_revisions(argv->nr, argv->v, &rev, &s_r_opt);\n \tif (prepare_revision_walk(&rev))\n \t\tdie(_(\"revision walk setup failed\"));\n \n\nbase-commit: 3a238e539bcdfe3f9eb5010fd218640c1b499f7a\n-- \ngitgitgadget\n"},{"id":"405086","messageId":"pull.725.v2.git.1599370473141.gitgitgadget@gmail.com","threadId":"54188","inReplyTo":"pull.725.git.1599231196975.gitgitgadget@gmail.com","subject":"[PATCH v2] submodule: suppress checking for file name ambiguity for object ids","fromName":"Orgad Shaneh via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-09-06T05:34:32Z","receivedAt":"2020-09-06T05:34:44Z","isPatch":true,"sender":{"key":"orgads@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1246544?v=4"},"body":"From: Orgad Shaneh <orgads@gmail.com>\n\nThe argv argument of collect_changed_submodules() contains obly object ids\n(the objects references of all the refs).\n\nNotify setup_revisions() that the input is not filenames by passing\nassume_dashdash, so it can avoid redundant stat for each ref.\n\nA better improvement would be to pass oid_array instead of stringified argv,\nbut that will require a larger change, which can be done later.\n\nSigned-off-by: Orgad Shaneh <orgads@gmail.com>\n---\n    submodule: suppress checking for file name ambiguity for object ids\n    \n    The argv argument of collect_changed_submodules() contains obly object\n    ids (submodule references).\n    \n    Notify setup_revisions() that the input is not filenames by passing\n    assume_dashdash, so it can avoid redundant stat for each ref.\n    \n    A better improvement would be to pass oid_array instead of stringified\n    argv, but that will require a larger change, which can be done later.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-725%2Forgads%2Fsubmodule-not-filename-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-725/orgads/submodule-not-filename-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/725\n\nRange-diff vs v1:\n\n 1:  f12112cc88 ! 1:  501ce90e9a submodule: suppress checking for file name ambiguity for object ids\n     @@ Commit message\n          submodule: suppress checking for file name ambiguity for object ids\n      \n          The argv argument of collect_changed_submodules() contains obly object ids\n     -    (submodule references).\n     +    (the objects references of all the refs).\n      \n          Notify setup_revisions() that the input is not filenames by passing\n          assume_dashdash, so it can avoid redundant stat for each ref.\n\n\n submodule.c | 5 ++++-\n 1 file changed, 4 insertions(+), 1 deletion(-)\n\ndiff --git a/submodule.c b/submodule.c\nindex 3cbcf40dfc..9b5bfb12a3 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -840,9 +840,12 @@ static void collect_changed_submodules(struct repository *r,\n {\n \tstruct rev_info rev;\n \tconst struct commit *commit;\n+\tstruct setup_revision_opt s_r_opt = {\n+\t\t.assume_dashdash = 1,\n+\t};\n \n \trepo_init_revisions(r, &rev, NULL);\n-\tsetup_revisions(argv->nr, argv->v, &rev, NULL);\n+\tsetup_revisions(argv->nr, argv->v, &rev, &s_r_opt);\n \tif (prepare_revision_walk(&rev))\n \t\tdie(_(\"revision walk setup failed\"));\n \n\nbase-commit: 3a238e539bcdfe3f9eb5010fd218640c1b499f7a\n-- \ngitgitgadget\n"},{"id":"405095","messageId":"CAGHpTBKduYnWymtCYR0AAdYy4rhXZgQkrUiHu59bpNX5UDEYfg@mail.gmail.com","threadId":"54188","inReplyTo":"pull.725.v2.git.1599370473141.gitgitgadget@gmail.com","subject":"Re: [PATCH v2] submodule: suppress checking for file name ambiguity for object ids","fromName":"Orgad Shaneh","fromEmail":"orgads@gmail.com","sentAt":"2020-09-06T19:25:28Z","receivedAt":"2020-09-06T19:25:47Z","isPatch":true,"sender":{"key":"orgads@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1246544?v=4"},"body":"On Sun, Sep 6, 2020 at 8:34 AM Orgad Shaneh via GitGitGadget\n<gitgitgadget@gmail.com> wrote:\n>\n> From: Orgad Shaneh <orgads@gmail.com>\n>\n> The argv argument of collect_changed_submodules() contains obly object ids\n> (the objects references of all the refs).\n>\n> Notify setup_revisions() that the input is not filenames by passing\n> assume_dashdash, so it can avoid redundant stat for each ref.\n>\n> A better improvement would be to pass oid_array instead of stringified argv,\n> but that will require a larger change, which can be done later.\n\nI'm wondering if it would be possible to track all the commits that\nwere received\nvia the transport, instead of resolving them by ref changes, because resolving\nfrom refs requires excluding all the previously-known refs, which can be a lot.\nOur repository has ~35K tags, and I believe there are larger repos out there...\n\nWhat do you say?\n\n- Orgad\n"},{"id":"405096","messageId":"pull.725.v3.git.1599425259773.gitgitgadget@gmail.com","threadId":"54188","inReplyTo":"pull.725.v2.git.1599370473141.gitgitgadget@gmail.com","subject":"[PATCH v3] submodule: suppress checking for file name and ref ambiguity for object ids","fromName":"Orgad Shaneh via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-09-06T20:47:39Z","receivedAt":"2020-09-06T20:47:49Z","isPatch":true,"sender":{"key":"orgads@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1246544?v=4"},"body":"From: Orgad Shaneh <orgads@gmail.com>\n\nThe argv argument of collect_changed_submodules() contains obly object ids\n(the objects references of all the refs).\n\nNotify setup_revisions() that the input is not filenames by passing\nassume_dashdash, so it can avoid redundant stat for each ref.\n\nAlso suppress refname_ambiguity flag to avoid filesystem lookups for\neach object. Similar logic can be found in cat-file, pack-objects and more.\n\nSigned-off-by: Orgad Shaneh <orgads@gmail.com>\n---\n    submodule: suppress checking for file name and ref ambiguity for object\n    ids\n    \n    The argv argument of collect_changed_submodules() contains obly object\n    ids (submodule references).\n    \n    Notify setup_revisions() that the input is not filenames by passing\n    assume_dashdash, so it can avoid redundant stat for each ref.\n    \n    A better improvement would be to pass oid_array instead of stringified\n    argv, but that will require a larger change, which can be done later.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-725%2Forgads%2Fsubmodule-not-filename-v3\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-725/orgads/submodule-not-filename-v3\nPull-Request: https://github.com/gitgitgadget/git/pull/725\n\nRange-diff vs v2:\n\n 1:  501ce90e9a ! 1:  128a7244f9 submodule: suppress checking for file name ambiguity for object ids\n     @@ Metadata\n      Author: Orgad Shaneh <orgads@gmail.com>\n      \n       ## Commit message ##\n     -    submodule: suppress checking for file name ambiguity for object ids\n     +    submodule: suppress checking for file name and ref ambiguity for object ids\n      \n          The argv argument of collect_changed_submodules() contains obly object ids\n          (the objects references of all the refs).\n     @@ Commit message\n          Notify setup_revisions() that the input is not filenames by passing\n          assume_dashdash, so it can avoid redundant stat for each ref.\n      \n     -    A better improvement would be to pass oid_array instead of stringified argv,\n     -    but that will require a larger change, which can be done later.\n     +    Also suppress refname_ambiguity flag to avoid filesystem lookups for\n     +    each object. Similar logic can be found in cat-file, pack-objects and more.\n      \n          Signed-off-by: Orgad Shaneh <orgads@gmail.com>\n      \n     @@ submodule.c: static void collect_changed_submodules(struct repository *r,\n       {\n       \tstruct rev_info rev;\n       \tconst struct commit *commit;\n     ++\tint save_warning;\n      +\tstruct setup_revision_opt s_r_opt = {\n      +\t\t.assume_dashdash = 1,\n      +\t};\n       \n     ++\tsave_warning = warn_on_object_refname_ambiguity;\n     ++\twarn_on_object_refname_ambiguity = 0;\n       \trepo_init_revisions(r, &rev, NULL);\n      -\tsetup_revisions(argv->nr, argv->v, &rev, NULL);\n      +\tsetup_revisions(argv->nr, argv->v, &rev, &s_r_opt);\n     ++\twarn_on_object_refname_ambiguity = save_warning;\n       \tif (prepare_revision_walk(&rev))\n       \t\tdie(_(\"revision walk setup failed\"));\n       \n\n\n submodule.c | 9 ++++++++-\n 1 file changed, 8 insertions(+), 1 deletion(-)\n\ndiff --git a/submodule.c b/submodule.c\nindex 3cbcf40dfc..e48710e423 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -840,9 +840,16 @@ static void collect_changed_submodules(struct repository *r,\n {\n \tstruct rev_info rev;\n \tconst struct commit *commit;\n+\tint save_warning;\n+\tstruct setup_revision_opt s_r_opt = {\n+\t\t.assume_dashdash = 1,\n+\t};\n \n+\tsave_warning = warn_on_object_refname_ambiguity;\n+\twarn_on_object_refname_ambiguity = 0;\n \trepo_init_revisions(r, &rev, NULL);\n-\tsetup_revisions(argv->nr, argv->v, &rev, NULL);\n+\tsetup_revisions(argv->nr, argv->v, &rev, &s_r_opt);\n+\twarn_on_object_refname_ambiguity = save_warning;\n \tif (prepare_revision_walk(&rev))\n \t\tdie(_(\"revision walk setup failed\"));\n \n\nbase-commit: 3a238e539bcdfe3f9eb5010fd218640c1b499f7a\n-- \ngitgitgadget\n"},{"id":"405097","messageId":"pull.725.v4.git.1599425636107.gitgitgadget@gmail.com","threadId":"54188","inReplyTo":"pull.725.v3.git.1599425259773.gitgitgadget@gmail.com","subject":"[PATCH v4] submodule: suppress checking for file name and ref ambiguity for object ids","fromName":"Orgad Shaneh via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-09-06T20:53:55Z","receivedAt":"2020-09-06T20:54:05Z","isPatch":true,"sender":{"key":"orgads@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1246544?v=4"},"body":"From: Orgad Shaneh <orgads@gmail.com>\n\nThe argv argument of collect_changed_submodules() contains obly object ids\n(the objects references of all the refs).\n\nNotify setup_revisions() that the input is not filenames by passing\nassume_dashdash, so it can avoid redundant stat for each ref.\n\nAlso suppress refname_ambiguity flag to avoid filesystem lookups for\neach object. Similar logic can be found in cat-file, pack-objects and more.\n\nThis change reduces the time for git fetch in my repo from 25s to 6s.\n\nSigned-off-by: Orgad Shaneh <orgads@gmail.com>\n---\n    submodule: suppress checking for file name and ref ambiguity for object\n    ids\n    \n    The argv argument of collect_changed_submodules() contains obly object\n    ids (submodule references).\n    \n    Notify setup_revisions() that the input is not filenames by passing\n    assume_dashdash, so it can avoid redundant stat for each ref.\n    \n    A better improvement would be to pass oid_array instead of stringified\n    argv, but that will require a larger change, which can be done later.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-725%2Forgads%2Fsubmodule-not-filename-v4\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-725/orgads/submodule-not-filename-v4\nPull-Request: https://github.com/gitgitgadget/git/pull/725\n\nRange-diff vs v3:\n\n 1:  128a7244f9 ! 1:  7134a87921 submodule: suppress checking for file name and ref ambiguity for object ids\n     @@ Commit message\n          Also suppress refname_ambiguity flag to avoid filesystem lookups for\n          each object. Similar logic can be found in cat-file, pack-objects and more.\n      \n     +    This change reduces the time for git fetch in my repo from 25s to 6s.\n     +\n          Signed-off-by: Orgad Shaneh <orgads@gmail.com>\n      \n       ## submodule.c ##\n\n\n submodule.c | 9 ++++++++-\n 1 file changed, 8 insertions(+), 1 deletion(-)\n\ndiff --git a/submodule.c b/submodule.c\nindex 3cbcf40dfc..e48710e423 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -840,9 +840,16 @@ static void collect_changed_submodules(struct repository *r,\n {\n \tstruct rev_info rev;\n \tconst struct commit *commit;\n+\tint save_warning;\n+\tstruct setup_revision_opt s_r_opt = {\n+\t\t.assume_dashdash = 1,\n+\t};\n \n+\tsave_warning = warn_on_object_refname_ambiguity;\n+\twarn_on_object_refname_ambiguity = 0;\n \trepo_init_revisions(r, &rev, NULL);\n-\tsetup_revisions(argv->nr, argv->v, &rev, NULL);\n+\tsetup_revisions(argv->nr, argv->v, &rev, &s_r_opt);\n+\twarn_on_object_refname_ambiguity = save_warning;\n \tif (prepare_revision_walk(&rev))\n \t\tdie(_(\"revision walk setup failed\"));\n \n\nbase-commit: 3a238e539bcdfe3f9eb5010fd218640c1b499f7a\n-- \ngitgitgadget\n"},{"id":"405098","messageId":"cc71351a-6792-5003-c3df-b60ab87e5220@ramsayjones.plus.com","threadId":"54188","inReplyTo":"pull.725.v4.git.1599425636107.gitgitgadget@gmail.com","subject":"Re: [PATCH v4] submodule: suppress checking for file name and ref ambiguity for object ids","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2020-09-06T20:59:58Z","receivedAt":"2020-09-06T21:00:05Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 06/09/2020 21:53, Orgad Shaneh via GitGitGadget wrote:\n> From: Orgad Shaneh <orgads@gmail.com>\n> \n> The argv argument of collect_changed_submodules() contains obly object ids\n\ns/obly/only/\n\nATB,\nRamsay Jones\n\n"},{"id":"405106","messageId":"xmqq7dt636sy.fsf@gitster.c.googlers.com","threadId":"54188","inReplyTo":"pull.725.v2.git.1599370473141.gitgitgadget@gmail.com","subject":"Re: [PATCH v2] submodule: suppress checking for file name ambiguity for object ids","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-09-06T21:59:41Z","receivedAt":"2020-09-06T21:59:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Orgad Shaneh via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n> From: Orgad Shaneh <orgads@gmail.com>\n>\n> The argv argument of collect_changed_submodules() contains obly object ids\n\nobly??? s/b/n/.\n\n> (the objects references of all the refs).\n>\n> Notify setup_revisions() that the input is not filenames by passing\n> assume_dashdash, so it can avoid redundant stat for each ref.\n>\n> A better improvement would be to pass oid_array instead of stringified argv,\n> but that will require a larger change, which can be done later.\n\nA naïve way is to append \"--\" to the argv strvec, but since 6d5b93f2\n(cherry-pick: do not expect file arguments, 2012-04-14) we made it\nunnecessary by introducing the flag.  This is exactly how the flag\nwas designed to be used.\n\nGood job.\n\nThanks.\n\n>\n> Signed-off-by: Orgad Shaneh <orgads@gmail.com>\n> ---\n>     submodule: suppress checking for file name ambiguity for object ids\n>     \n>     The argv argument of collect_changed_submodules() contains obly object\n>     ids (submodule references).\n>     \n>     Notify setup_revisions() that the input is not filenames by passing\n>     assume_dashdash, so it can avoid redundant stat for each ref.\n>     \n>     A better improvement would be to pass oid_array instead of stringified\n>     argv, but that will require a larger change, which can be done later.\n>\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-725%2Forgads%2Fsubmodule-not-filename-v2\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-725/orgads/submodule-not-filename-v2\n> Pull-Request: https://github.com/gitgitgadget/git/pull/725\n>\n> Range-diff vs v1:\n>\n>  1:  f12112cc88 ! 1:  501ce90e9a submodule: suppress checking for file name ambiguity for object ids\n>      @@ Commit message\n>           submodule: suppress checking for file name ambiguity for object ids\n>       \n>           The argv argument of collect_changed_submodules() contains obly object ids\n>      -    (submodule references).\n>      +    (the objects references of all the refs).\n>       \n>           Notify setup_revisions() that the input is not filenames by passing\n>           assume_dashdash, so it can avoid redundant stat for each ref.\n>\n>\n>  submodule.c | 5 ++++-\n>  1 file changed, 4 insertions(+), 1 deletion(-)\n>\n> diff --git a/submodule.c b/submodule.c\n> index 3cbcf40dfc..9b5bfb12a3 100644\n> --- a/submodule.c\n> +++ b/submodule.c\n> @@ -840,9 +840,12 @@ static void collect_changed_submodules(struct repository *r,\n>  {\n>  \tstruct rev_info rev;\n>  \tconst struct commit *commit;\n> +\tstruct setup_revision_opt s_r_opt = {\n> +\t\t.assume_dashdash = 1,\n> +\t};\n>  \n>  \trepo_init_revisions(r, &rev, NULL);\n> -\tsetup_revisions(argv->nr, argv->v, &rev, NULL);\n> +\tsetup_revisions(argv->nr, argv->v, &rev, &s_r_opt);\n>  \tif (prepare_revision_walk(&rev))\n>  \t\tdie(_(\"revision walk setup failed\"));\n>  \n>\n> base-commit: 3a238e539bcdfe3f9eb5010fd218640c1b499f7a\n"}]}