Re: [RFC PATCH] fetch-pack: space out sent "haves" in negotiation

From: Junio C Hamano <gitster@pobox.com>
Date: May 23, 2018, 03:42 UTC
Message-ID: <xmqqwovvujfx.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <20180521204340.260572-1-jonathantanmy@google.com>

Jonathan Tan <jonathantanmy@google.com> writes:

Show 8 quoted lines

>  Makefile           |   1 +
>  fetch-negotiator.c | 309 +++++++++++++++++++++++++++++++++++++++++++++
>  fetch-negotiator.h |  40 ++++++
>  fetch-pack.c       | 174 ++++++-------------------
>  object.h           |   1 +
>  5 files changed, 392 insertions(+), 133 deletions(-)
>  create mode 100644 fetch-negotiator.c
>  create mode 100644 fetch-negotiator.h

Somehow this feels more like a WIP than RFC, primarily for two reasons. It was unclear what "edge" computation is trying to do; it seems way under-explained, especially the part that takes min-max while. merging two candidates.

It also was unclear if this should be organized as a "take it or leave it" patch like this one, or eventually should be split into multiple steps when it gets polished enough to be considered for application, the early ones introducing a separate negotiator module without changing the common ancestor discovery algorithm at all, with later steps refining that negotiator and add more efficient common ancestor discovery process.

Show 23 quoted lines

> diff --git a/Makefile b/Makefile
> index ad880d1fc5..8bbedfa521 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -859,6 +859,7 @@ LIB_OBJS += ewah/ewah_bitmap.o
>  LIB_OBJS += ewah/ewah_io.o
>  LIB_OBJS += ewah/ewah_rlw.o
>  LIB_OBJS += exec-cmd.o
> +LIB_OBJS += fetch-negotiator.o
>  LIB_OBJS += fetch-object.o
>  LIB_OBJS += fetch-pack.o
>  LIB_OBJS += fsck.o
> diff --git a/fetch-negotiator.c b/fetch-negotiator.c
> new file mode 100644
> index 0000000000..58975e1c37
> --- /dev/null
> +++ b/fetch-negotiator.c
> @@ -0,0 +1,309 @@
> +#include "cache.h"
> +#include "commit.h"
> +#include "fetch-negotiator.h"
> +
> +#define NO_THE_INDEX_COMPATIBILITY_MACROS

A totally unrelated tangent, but will we also benefit from NO_THE_REPO_COMPATIBILITY_MACROS eventually?