git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
* [PATCH] submodule: suppress checking for file name ambiguity for object ids
@ 2020-09-04 14:53 Orgad Shaneh via GitGitGadget
  2020-09-06  5:34 ` [PATCH v2] " Orgad Shaneh via GitGitGadget
  0 siblings, 1 reply; 7+ messages in thread
From: Orgad Shaneh via GitGitGadget @ 2020-09-04 14:53 UTC (permalink / raw)
  To: git; +Cc: Orgad Shaneh, Orgad Shaneh

From: Orgad Shaneh <orgads@gmail.com>

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.

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-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-725/orgads/submodule-not-filename-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/725

 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
-- 
gitgitgadget

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v2] submodule: suppress checking for file name ambiguity for object ids
  2020-09-04 14:53 [PATCH] submodule: suppress checking for file name ambiguity for object ids Orgad Shaneh via GitGitGadget
@ 2020-09-06  5:34 ` Orgad Shaneh via GitGitGadget
  2020-09-06 19:25   ` Orgad Shaneh
                     ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Orgad Shaneh via GitGitGadget @ 2020-09-06  5:34 UTC (permalink / raw)
  To: git; +Cc: Orgad Shaneh, Orgad Shaneh

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(-)

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
-- 
gitgitgadget

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] submodule: suppress checking for file name ambiguity for object ids
  2020-09-06  5:34 ` [PATCH v2] " Orgad Shaneh via GitGitGadget
@ 2020-09-06 19:25   ` Orgad Shaneh
  2020-09-06 20:47   ` [PATCH v3] submodule: suppress checking for file name and ref " Orgad Shaneh via GitGitGadget
  2020-09-06 21:59   ` [PATCH v2] submodule: suppress checking for file name " Junio C Hamano
  2 siblings, 0 replies; 7+ messages in thread
From: Orgad Shaneh @ 2020-09-06 19:25 UTC (permalink / raw)
  To: Orgad Shaneh via GitGitGadget; +Cc: git

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v3] submodule: suppress checking for file name and ref ambiguity for object ids
  2020-09-06  5:34 ` [PATCH v2] " Orgad Shaneh via GitGitGadget
  2020-09-06 19:25   ` Orgad Shaneh
@ 2020-09-06 20:47   ` Orgad Shaneh via GitGitGadget
  2020-09-06 20:53     ` [PATCH v4] " Orgad Shaneh via GitGitGadget
  2020-09-06 21:59   ` [PATCH v2] submodule: suppress checking for file name " Junio C Hamano
  2 siblings, 1 reply; 7+ messages in thread
From: Orgad Shaneh via GitGitGadget @ 2020-09-06 20:47 UTC (permalink / raw)
  To: git; +Cc: Orgad Shaneh, Orgad Shaneh, Orgad Shaneh

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.

Also suppress refname_ambiguity flag to avoid filesystem lookups for
each object. Similar logic can be found in cat-file, pack-objects and more.

Signed-off-by: Orgad Shaneh <orgads@gmail.com>
---
    submodule: suppress checking for file name and ref 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-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-725/orgads/submodule-not-filename-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/725

Range-diff vs v2:

 1:  501ce90e9a ! 1:  128a7244f9 submodule: suppress checking for file name ambiguity for object ids
     @@ Metadata
      Author: Orgad Shaneh <orgads@gmail.com>
      
       ## Commit message ##
     -    submodule: suppress checking for file name ambiguity for object ids
     +    submodule: suppress checking for file name and ref ambiguity for object ids
      
          The argv argument of collect_changed_submodules() contains obly object ids
          (the objects references of all the refs).
     @@ Commit message
          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.
     +    Also suppress refname_ambiguity flag to avoid filesystem lookups for
     +    each object. Similar logic can be found in cat-file, pack-objects and more.
      
          Signed-off-by: Orgad Shaneh <orgads@gmail.com>
      
     @@ submodule.c: static void collect_changed_submodules(struct repository *r,
       {
       	struct rev_info rev;
       	const struct commit *commit;
     ++	int save_warning;
      +	struct setup_revision_opt s_r_opt = {
      +		.assume_dashdash = 1,
      +	};
       
     ++	save_warning = warn_on_object_refname_ambiguity;
     ++	warn_on_object_refname_ambiguity = 0;
       	repo_init_revisions(r, &rev, NULL);
      -	setup_revisions(argv->nr, argv->v, &rev, NULL);
      +	setup_revisions(argv->nr, argv->v, &rev, &s_r_opt);
     ++	warn_on_object_refname_ambiguity = save_warning;
       	if (prepare_revision_walk(&rev))
       		die(_("revision walk setup failed"));
       


 submodule.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

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

base-commit: 3a238e539bcdfe3f9eb5010fd218640c1b499f7a
-- 
gitgitgadget

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v4] submodule: suppress checking for file name and ref ambiguity for object ids
  2020-09-06 20:47   ` [PATCH v3] submodule: suppress checking for file name and ref " Orgad Shaneh via GitGitGadget
@ 2020-09-06 20:53     ` Orgad Shaneh via GitGitGadget
  2020-09-06 20:59       ` Ramsay Jones
  0 siblings, 1 reply; 7+ messages in thread
From: Orgad Shaneh via GitGitGadget @ 2020-09-06 20:53 UTC (permalink / raw)
  To: git; +Cc: Orgad Shaneh, Orgad Shaneh, Orgad Shaneh

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.

Also suppress refname_ambiguity flag to avoid filesystem lookups for
each object. Similar logic can be found in cat-file, pack-objects and more.

This change reduces the time for git fetch in my repo from 25s to 6s.

Signed-off-by: Orgad Shaneh <orgads@gmail.com>
---
    submodule: suppress checking for file name and ref 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-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-725/orgads/submodule-not-filename-v4
Pull-Request: https://github.com/gitgitgadget/git/pull/725

Range-diff vs v3:

 1:  128a7244f9 ! 1:  7134a87921 submodule: suppress checking for file name and ref ambiguity for object ids
     @@ Commit message
          Also suppress refname_ambiguity flag to avoid filesystem lookups for
          each object. Similar logic can be found in cat-file, pack-objects and more.
      
     +    This change reduces the time for git fetch in my repo from 25s to 6s.
     +
          Signed-off-by: Orgad Shaneh <orgads@gmail.com>
      
       ## submodule.c ##


 submodule.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

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

base-commit: 3a238e539bcdfe3f9eb5010fd218640c1b499f7a
-- 
gitgitgadget

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v4] submodule: suppress checking for file name and ref ambiguity for object ids
  2020-09-06 20:53     ` [PATCH v4] " Orgad Shaneh via GitGitGadget
@ 2020-09-06 20:59       ` Ramsay Jones
  0 siblings, 0 replies; 7+ messages in thread
From: Ramsay Jones @ 2020-09-06 20:59 UTC (permalink / raw)
  To: Orgad Shaneh via GitGitGadget, git; +Cc: Orgad Shaneh



On 06/09/2020 21:53, Orgad Shaneh via GitGitGadget wrote:
> From: Orgad Shaneh <orgads@gmail.com>
> 
> The argv argument of collect_changed_submodules() contains obly object ids

s/obly/only/

ATB,
Ramsay Jones


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] submodule: suppress checking for file name ambiguity for object ids
  2020-09-06  5:34 ` [PATCH v2] " Orgad Shaneh via GitGitGadget
  2020-09-06 19:25   ` Orgad Shaneh
  2020-09-06 20:47   ` [PATCH v3] submodule: suppress checking for file name and ref " Orgad Shaneh via GitGitGadget
@ 2020-09-06 21:59   ` Junio C Hamano
  2 siblings, 0 replies; 7+ messages in thread
From: Junio C Hamano @ 2020-09-06 21:59 UTC (permalink / raw)
  To: Orgad Shaneh via GitGitGadget; +Cc: git, Orgad Shaneh

"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

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2020-09-06 21:59 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-04 14:53 [PATCH] submodule: suppress checking for file name ambiguity for object ids Orgad Shaneh via GitGitGadget
2020-09-06  5:34 ` [PATCH v2] " Orgad Shaneh via GitGitGadget
2020-09-06 19:25   ` Orgad Shaneh
2020-09-06 20:47   ` [PATCH v3] submodule: suppress checking for file name and ref " Orgad Shaneh via GitGitGadget
2020-09-06 20:53     ` [PATCH v4] " Orgad Shaneh via GitGitGadget
2020-09-06 20:59       ` Ramsay Jones
2020-09-06 21:59   ` [PATCH v2] submodule: suppress checking for file name " Junio C Hamano

git@vger.kernel.org list mirror (unofficial, one of many)

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V1 git git/ https://public-inbox.org/git \
		git@vger.kernel.org
	public-inbox-index git

Example config snippet for mirrors.
Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://7fh6tueqddpjyxjmgtdiueylzoqt6pt7hec3pukyptlmohoowvhde4yd.onion/inbox.comp.version-control.git
	nntp://ie5yzdi7fg72h7s4sdcztq5evakq23rdt33mfyfcddc5u3ndnw24ogqd.onion/inbox.comp.version-control.git
	nntp://4uok3hntl7oi7b4uf4rtfwefqeexfzil2w6kgk2jn5z2f764irre7byd.onion/inbox.comp.version-control.git
	nntp://news.gmane.io/gmane.comp.version-control.git
 note: .onion URLs require Tor: https://www.torproject.org/

code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git