On Fri, Oct 7, 2016 at 8:06 AM, Heiko Voigt <hvoigt@xxxxxxxxxx> wrote: > We run a command for each sha1 change in a submodule. This is > unnecessary since we can simply batch all sha1's we want to check into > one command. Lets do it so we can speedup the check when many submodule > changes are in need of checking. > > Signed-off-by: Heiko Voigt <hvoigt@xxxxxxxxxx> > --- > submodule.c | 63 +++++++++++++++++++++++++++++++++---------------------------- > 1 file changed, 34 insertions(+), 29 deletions(-) > > diff --git a/submodule.c b/submodule.c > index 5044afc2f8..a05c2a34b1 100644 > --- a/submodule.c > +++ b/submodule.c > @@ -529,27 +529,49 @@ static int append_hash_to_argv(const unsigned char sha1[20], void *data) > return 0; > } > > -static int submodule_needs_pushing(const char *path, const unsigned char sha1[20]) > +static int check_has_hash(const unsigned char sha1[20], void *data) > { > - if (add_submodule_odb(path) || !lookup_commit_reference(sha1)) > + int *has_hash = (int *) data; > + > + if (!lookup_commit_reference(sha1)) > + *has_hash = 0; > + > + return 0; > +} > + > +static int submodule_has_hashes(const char *path, struct sha1_array *hashes) > +{ > + int has_hash = 1; > + > + if (add_submodule_odb(path)) > + return 0; > + > + sha1_array_for_each_unique(hashes, check_has_hash, &has_hash); > + return has_hash; > +} > + > +static int submodule_needs_pushing(const char *path, struct sha1_array *hashes) > +{ > + if (!submodule_has_hashes(path, hashes)) So the above is an implicit lookup already, but we did that before, too, so it's fine. > @@ -658,13 +665,11 @@ int find_unpushed_submodules(struct sha1_array *hashes, > argv_array_clear(&argv); > > for (i = 0; i < submodules.nr; i++) { > - struct string_list_item *item = &submodules.items[i]; > - struct collect_submodule_from_sha1s_data data; > - data.submodule_path = item->string; > - data.needs_pushing = needs_pushing; > - sha1_array_for_each_unique((struct sha1_array *) item->util, > - collect_submodules_from_sha1s, > - &data); > + struct string_list_item *submodule = &submodules.items[i]; > + struct sha1_array *hashes = (struct sha1_array *) submodule->util; > + > + if (submodule_needs_pushing(submodule->string, hashes)) > + string_list_insert(needs_pushing, submodule->string); That makes sense. Thanks! Stefan