submodule: suppress checking for file name ambiguity for object ids

From: Orgad Shaneh <[email protected]>

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.

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

index 3cbcf40dfc..9b5bfb12a3 100644
@@ -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

From: Orgad Shaneh <[email protected]>

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.

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

index 3cbcf40dfc..9b5bfb12a3 100644
@@ -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

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

From: Orgad Shaneh <[email protected]>

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.

---
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 <[email protected]>

## 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.

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

index 3cbcf40dfc..e48710e423 100644
@@ -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

From: Orgad Shaneh <[email protected]>

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.

---
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.
+

## submodule.c ##

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

index 3cbcf40dfc..e48710e423 100644
@@ -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

s/obly/only/

ATB,
Ramsay Jones

obly??? s/b/n/.

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.