diff mbox series

[v2] submodule: suppress checking for file name ambiguity for object ids

Message ID pull.725.v2.git.1599370473141.gitgitgadget@gmail.com (mailing list archive)
State New, archived
Headers show
Series [v2] submodule: suppress checking for file name ambiguity for object ids | expand

Commit Message

Johannes Schindelin via GitGitGadget Sept. 6, 2020, 5:34 a.m. UTC
From: Orgad Shaneh <orgads@gmail.com>

The argv argument of collect_changed_submodules() contains obly object ids
(the objects references of all the refs).

Notify setup_revisions() that the input is not filenames by passing
assume_dashdash, so it can avoid redundant stat for each ref.

A better improvement would be to pass oid_array instead of stringified argv,
but that will require a larger change, which can be done later.

Signed-off-by: Orgad Shaneh <orgads@gmail.com>
---
    submodule: suppress checking for file name ambiguity for object ids
    
    The argv argument of collect_changed_submodules() contains obly object
    ids (submodule references).
    
    Notify setup_revisions() that the input is not filenames by passing
    assume_dashdash, so it can avoid redundant stat for each ref.
    
    A better improvement would be to pass oid_array instead of stringified
    argv, but that will require a larger change, which can be done later.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-725%2Forgads%2Fsubmodule-not-filename-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-725/orgads/submodule-not-filename-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/725

Range-diff vs v1:

 1:  f12112cc88 ! 1:  501ce90e9a submodule: suppress checking for file name ambiguity for object ids
     @@ Commit message
          submodule: suppress checking for file name ambiguity for object ids
      
          The argv argument of collect_changed_submodules() contains obly object ids
     -    (submodule references).
     +    (the objects references of all the refs).
      
          Notify setup_revisions() that the input is not filenames by passing
          assume_dashdash, so it can avoid redundant stat for each ref.


 submodule.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)


base-commit: 3a238e539bcdfe3f9eb5010fd218640c1b499f7a

Comments

Orgad Shaneh Sept. 6, 2020, 7:25 p.m. UTC | #1
On Sun, Sep 6, 2020 at 8:34 AM Orgad Shaneh via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> From: Orgad Shaneh <orgads@gmail.com>
>
> The argv argument of collect_changed_submodules() contains obly object ids
> (the objects references of all the refs).
>
> Notify setup_revisions() that the input is not filenames by passing
> assume_dashdash, so it can avoid redundant stat for each ref.
>
> A better improvement would be to pass oid_array instead of stringified argv,
> but that will require a larger change, which can be done later.

I'm wondering if it would be possible to track all the commits that
were received
via the transport, instead of resolving them by ref changes, because resolving
from refs requires excluding all the previously-known refs, which can be a lot.
Our repository has ~35K tags, and I believe there are larger repos out there...

What do you say?

- Orgad
Junio C Hamano Sept. 6, 2020, 9:59 p.m. UTC | #2
"Orgad Shaneh via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Orgad Shaneh <orgads@gmail.com>
>
> The argv argument of collect_changed_submodules() contains obly object ids

obly??? s/b/n/.

> (the objects references of all the refs).
>
> Notify setup_revisions() that the input is not filenames by passing
> assume_dashdash, so it can avoid redundant stat for each ref.
>
> A better improvement would be to pass oid_array instead of stringified argv,
> but that will require a larger change, which can be done later.

A naïve way is to append "--" to the argv strvec, but since 6d5b93f2
(cherry-pick: do not expect file arguments, 2012-04-14) we made it
unnecessary by introducing the flag.  This is exactly how the flag
was designed to be used.

Good job.

Thanks.

>
> Signed-off-by: Orgad Shaneh <orgads@gmail.com>
> ---
>     submodule: suppress checking for file name ambiguity for object ids
>     
>     The argv argument of collect_changed_submodules() contains obly object
>     ids (submodule references).
>     
>     Notify setup_revisions() that the input is not filenames by passing
>     assume_dashdash, so it can avoid redundant stat for each ref.
>     
>     A better improvement would be to pass oid_array instead of stringified
>     argv, but that will require a larger change, which can be done later.
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-725%2Forgads%2Fsubmodule-not-filename-v2
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-725/orgads/submodule-not-filename-v2
> Pull-Request: https://github.com/gitgitgadget/git/pull/725
>
> Range-diff vs v1:
>
>  1:  f12112cc88 ! 1:  501ce90e9a submodule: suppress checking for file name ambiguity for object ids
>      @@ Commit message
>           submodule: suppress checking for file name ambiguity for object ids
>       
>           The argv argument of collect_changed_submodules() contains obly object ids
>      -    (submodule references).
>      +    (the objects references of all the refs).
>       
>           Notify setup_revisions() that the input is not filenames by passing
>           assume_dashdash, so it can avoid redundant stat for each ref.
>
>
>  submodule.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/submodule.c b/submodule.c
> index 3cbcf40dfc..9b5bfb12a3 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -840,9 +840,12 @@ static void collect_changed_submodules(struct repository *r,
>  {
>  	struct rev_info rev;
>  	const struct commit *commit;
> +	struct setup_revision_opt s_r_opt = {
> +		.assume_dashdash = 1,
> +	};
>  
>  	repo_init_revisions(r, &rev, NULL);
> -	setup_revisions(argv->nr, argv->v, &rev, NULL);
> +	setup_revisions(argv->nr, argv->v, &rev, &s_r_opt);
>  	if (prepare_revision_walk(&rev))
>  		die(_("revision walk setup failed"));
>  
>
> base-commit: 3a238e539bcdfe3f9eb5010fd218640c1b499f7a
diff mbox series

Patch

diff --git a/submodule.c b/submodule.c
index 3cbcf40dfc..9b5bfb12a3 100644
--- a/submodule.c
+++ b/submodule.c
@@ -840,9 +840,12 @@  static void collect_changed_submodules(struct repository *r,
 {
 	struct rev_info rev;
 	const struct commit *commit;
+	struct setup_revision_opt s_r_opt = {
+		.assume_dashdash = 1,
+	};
 
 	repo_init_revisions(r, &rev, NULL);
-	setup_revisions(argv->nr, argv->v, &rev, NULL);
+	setup_revisions(argv->nr, argv->v, &rev, &s_r_opt);
 	if (prepare_revision_walk(&rev))
 		die(_("revision walk setup failed"));