Re: [PATCH v3 6/6] commit-graph: remove Future Work section

From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, sandals@crustytoothpaste.net,
	Junio C Hamano <gitster@pobox.com>,
	Derrick Stolee <dstolee@microsoft.com>
Subject: Re: [PATCH v3 6/6] commit-graph: remove Future Work section
Date: Wed, 01 May 2019 16:58:45 +0200	[thread overview]
Message-ID: <87o94mql0a.fsf@evledraar.gmail.com> (raw)
In-Reply-To: <e72bca6c784c5fc727ae4cf661526eec346c478c.1556716273.git.gitgitgadget@gmail.com>

On Wed, May 01 2019, Derrick Stolee via GitGitGadget wrote:

> The commit-graph feature began with a long list of planned
> benefits, most of which are now complete. The future work
> section has only a few items left.
>
> As for making more algorithms aware of generation numbers,
> some are only waiting for generation number v2 to ensure the
> performance matches the existing behavior using commit date.
>
> It is unlikely that we will ever send a commit-graph file
> as part of the protocol, since we would need to verify the
> data, and that is as expensive as writing a commit-graph from
> scratch. If we want to start trusting remote content, then
> that item can be investigated again.

My best of 3 times for "write" followed by "verify" on linux.git are
8.7/7.9 real/user for "write" and 5.2/4.9 real/user for "write".

So that's a reduction of ~40%. I have another big in-house repo where I
get similar numbers of 17/16 for "write" and 10/9 for "verify". Both for
a commit-graph file on the order of 50MB where it would be quicker for
me to download and verify it if the protocol supported it.

I'm not clamoring to make it part of the protocol, but the claim that
"verify" needs to do the equivalent of "write" seems to be demonstrably
wrong, or perhaps "verify" isn't doing all the work it should be doing?

> While there is more work to be done on the feature, having
> a section of the docs devoted to a TODO list is wasteful and
> hard to keep up-to-date.

Agreed, whatever we decide to do in the future I think it makes sense to
remove this section from the docs, although perhaps the commit message
should be amended per the above :)

> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
> ---
>  Documentation/technical/commit-graph.txt | 17 -----------------
>  1 file changed, 17 deletions(-)
>
> diff --git a/Documentation/technical/commit-graph.txt b/Documentation/technical/commit-graph.txt
> index 7805b0968c..fb53341d5e 100644
> --- a/Documentation/technical/commit-graph.txt
> +++ b/Documentation/technical/commit-graph.txt
> @@ -127,23 +127,6 @@ Design Details
>    helpful for these clones, anyway. The commit-graph will not be read or
>    written when shallow commits are present.
>
> -Future Work
> ------------
> -
> -- After computing and storing generation numbers, we must make graph
> -  walks aware of generation numbers to gain the performance benefits they
> -  enable. This will mostly be accomplished by swapping a commit-date-ordered
> -  priority queue with one ordered by generation number. The following
> -  operations are important candidates:
> -
> -    - 'log --topo-order'
> -    - 'tag --merged'
> -
> -- A server could provide a commit-graph file as part of the network protocol
> -  to avoid extra calculations by clients. This feature is only of benefit if
> -  the user is willing to trust the file, because verifying the file is correct
> -  is as hard as computing it from scratch.
> -
>  Related Links
>  -------------
>  [0] https://bugs.chromium.org/p/git/issues/detail?id=8