| Message ID | 474eb27d9c136fb69e961546004cfb531d722e2c.1585854639.git.jonathantanmy@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Restrict when prefetcing occurs | expand |
Jonathan Tan <jonathantanmy@google.com> writes: > There are 3 callers to promisor_remote_get_direct() that first check if > the number of objects to be fetched is equal to 0. Fold that check into > promisor_remote_get_direct(), and in doing so, be explicit as to what > promisor_remote_get_direct() does if oid_nr is 0 (it returns 0, success, > immediately). > > Signed-off-by: Jonathan Tan <jonathantanmy@google.com> > --- > builtin/index-pack.c | 5 ++--- > diff.c | 11 +++++------ > promisor-remote.c | 3 +++ > promisor-remote.h | 8 ++++++++ > unpack-trees.c | 5 ++--- > 5 files changed, 20 insertions(+), 12 deletions(-) Nice simplification. > +/* > + * Fetches all requested objects from all promisor remotes, trying them one at > + * a time until all objects are fetched. Returns 0 upon success, and non-zero > + * otherwise. Good. > + * If oid_nr is 0, this function returns 0 (success) immediately. Is this worth saying? If you ask to lazily grab 0 objects, it is probably clear that no object would be read before the helper returns. When oid_nr==0 you are allowed to pass oids==NULL, but otherwise, oids==NULL would be an error. Is that the kind of difference you wanted to point out, I wonder? > + */ > int promisor_remote_get_direct(struct repository *repo, > const struct object_id *oids, > int oid_nr);
> > + * If oid_nr is 0, this function returns 0 (success) immediately. > > Is this worth saying? If you ask to lazily grab 0 objects, it is > probably clear that no object would be read before the helper > returns. > > When oid_nr==0 you are allowed to pass oids==NULL, but otherwise, > oids==NULL would be an error. Is that the kind of difference you > wanted to point out, I wonder? Thanks for taking a look. Yes that was the difference I wanted to point out. After some thought, maybe I'll replace it with "oids points to an array of OIDs of size oid_nr. If oid_nr is 0, oids can be anything.".
diff --git a/builtin/index-pack.c b/builtin/index-pack.c index d967d188a3..f176dd28c8 100644 --- a/builtin/index-pack.c +++ b/builtin/index-pack.c @@ -1368,9 +1368,8 @@ static void fix_unresolved_deltas(struct hashfile *f) continue; oid_array_append(&to_fetch, &d->oid); } - if (to_fetch.nr) - promisor_remote_get_direct(the_repository, - to_fetch.oid, to_fetch.nr); + promisor_remote_get_direct(the_repository, + to_fetch.oid, to_fetch.nr); oid_array_clear(&to_fetch); } diff --git a/diff.c b/diff.c index 1010d806f5..f01b4d91b8 100644 --- a/diff.c +++ b/diff.c @@ -6520,12 +6520,11 @@ void diffcore_std(struct diff_options *options) add_if_missing(options->repo, &to_fetch, p->one); add_if_missing(options->repo, &to_fetch, p->two); } - if (to_fetch.nr) - /* - * NEEDSWORK: Consider deduplicating the OIDs sent. - */ - promisor_remote_get_direct(options->repo, - to_fetch.oid, to_fetch.nr); + /* + * NEEDSWORK: Consider deduplicating the OIDs sent. + */ + promisor_remote_get_direct(options->repo, + to_fetch.oid, to_fetch.nr); oid_array_clear(&to_fetch); } diff --git a/promisor-remote.c b/promisor-remote.c index 9f338c945f..2155dfe657 100644 --- a/promisor-remote.c +++ b/promisor-remote.c @@ -241,6 +241,9 @@ int promisor_remote_get_direct(struct repository *repo, int to_free = 0; int res = -1; + if (oid_nr == 0) + return 0; + promisor_remote_init(); for (r = promisors; r; r = r->next) { diff --git a/promisor-remote.h b/promisor-remote.h index 737bac3a33..6343c47d18 100644 --- a/promisor-remote.h +++ b/promisor-remote.h @@ -20,6 +20,14 @@ struct promisor_remote { void promisor_remote_reinit(void); struct promisor_remote *promisor_remote_find(const char *remote_name); int has_promisor_remote(void); + +/* + * Fetches all requested objects from all promisor remotes, trying them one at + * a time until all objects are fetched. Returns 0 upon success, and non-zero + * otherwise. + * + * If oid_nr is 0, this function returns 0 (success) immediately. + */ int promisor_remote_get_direct(struct repository *repo, const struct object_id *oids, int oid_nr); diff --git a/unpack-trees.c b/unpack-trees.c index f618a644ef..4c3191b947 100644 --- a/unpack-trees.c +++ b/unpack-trees.c @@ -423,9 +423,8 @@ static int check_updates(struct unpack_trees_options *o) continue; oid_array_append(&to_fetch, &ce->oid); } - if (to_fetch.nr) - promisor_remote_get_direct(the_repository, - to_fetch.oid, to_fetch.nr); + promisor_remote_get_direct(the_repository, + to_fetch.oid, to_fetch.nr); oid_array_clear(&to_fetch); } for (i = 0; i < index->cache_nr; i++) {
There are 3 callers to promisor_remote_get_direct() that first check if the number of objects to be fetched is equal to 0. Fold that check into promisor_remote_get_direct(), and in doing so, be explicit as to what promisor_remote_get_direct() does if oid_nr is 0 (it returns 0, success, immediately). Signed-off-by: Jonathan Tan <jonathantanmy@google.com> --- builtin/index-pack.c | 5 ++--- diff.c | 11 +++++------ promisor-remote.c | 3 +++ promisor-remote.h | 8 ++++++++ unpack-trees.c | 5 ++--- 5 files changed, 20 insertions(+), 12 deletions(-)