Jonathan Tan wrote:
> If a filter is specified, we do not need a full connectivity check on
> the contents of the packfile we just fetched; we only need to check that
> the objects referenced are promisor objects.
>
> This significantly speeds up fetches into repositories that have many
> promisor objects, because during the connectivity check, all promisor
> objects are enumerated (to mark them UNINTERESTING), and that takes a
> significant amount of time.
>
> Signed-off-by: Jonathan Tan <jonathantanmy@xxxxxxxxxx>
> ---
> For example, a local fetch was sped up from 6.63s to 3.39s. The bulk of
> the remaining time is spent in yet another connectivity check
> (fetch_refs -> check_exist_and_connected) prior to the fetch - that will
> hopefully be done in a subsequent patch.
Can this information (at least the speedup) be included in the comment
message?
Or even better, can we demonstrate the impact using a perf test?
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -906,8 +906,17 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
> url = xstrdup("foreign");
>
> if (!connectivity_checked) {
> + struct check_connected_options opt = CHECK_CONNECTED_INIT;
> +
> + if (filter_options.choice)
> + /*
> + * Since a filter is specified, objects indirectly
> + * referenced by refs are allowed to be absent.
> + */
> + opt.check_refs_are_promisor_objects_only = 1;
> +
> rm = ref_map;
> - if (check_connected(iterate_ref_map, &rm, NULL)) {
> + if (check_connected(iterate_ref_map, &rm, &opt)) {
> rc = error(_("%s did not send all necessary objects\n"), url);
> goto abort;
> }
Simple and sensible. With or without a change like described above,
Reviewed-by: Jonathan Nieder <jrnieder@xxxxxxxxx>
Thanks.
