Subject: Re: [PATCH v3 20/20] commit-graph: update design document

From: Jakub Narebski <[email-masked]>
Date: Sat, 02 Jun 2018 20:27:48 +0200

Derrick Stolee <[email-masked]> writes:

> The commit-graph feature is now integrated with 'fsck' and 'gc',
> so remove those items from the "Future Work" section of the
> commit-graph design document.

It is always nice to have such commit as a summary what was done in the
series, and to have up to date roadmap.

>
> Also remove the section on lazy-loading trees, as that was completed
> in an earlier patch series.

Admittedly, this part could have been sent in a separate patch at the
start of the series, but it doesn't matter at all; no need for extra
work.

>
> Signed-off-by: Derrick Stolee <[email-masked]>
> ---
>  Documentation/technical/commit-graph.txt | 22 ----------------------
>  1 file changed, 22 deletions(-)
>
> diff --git a/Documentation/technical/commit-graph.txt b/Documentation/technical/commit-graph.txt
> index e1a883eb46..c664acbd76 100644
> --- a/Documentation/technical/commit-graph.txt
> +++ b/Documentation/technical/commit-graph.txt
> @@ -118,9 +118,6 @@ Future Work
>  - The commit graph feature currently does not honor commit grafts. This can
>    be remedied by duplicating or refactoring the current graft logic.
>  
> -- The 'commit-graph' subcommand does not have a "verify" mode that is
> -  necessary for integration with fsck.
> -

All right.

>  - 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
> @@ -130,25 +127,6 @@ Future Work
>      - 'log --topo-order'
>      - 'tag --merged'
>  
> -- Currently, parse_commit_gently() requires filling in the root tree
> -  object for a commit. This passes through lookup_tree() and consequently
> -  lookup_object(). Also, it calls lookup_commit() when loading the parents.
> -  These method calls check the ODB for object existence, even if the
> -  consumer does not need the content. For example, we do not need the
> -  tree contents when computing merge bases. Now that commit parsing is
> -  removed from the computation time, these lookup operations are the
> -  slowest operations keeping graph walks from being fast. Consider
> -  loading these objects without verifying their existence in the ODB and
> -  only loading them fully when consumers need them. Consider a method
> -  such as "ensure_tree_loaded(commit)" that fully loads a tree before
> -  using commit->tree.

All right, this is about the change done in previous series.

> -
> -- The current design uses the 'commit-graph' subcommand to generate the graph.
> -  When this feature stabilizes enough to recommend to most users, we should
> -  add automatic graph writes to common operations that create many commits.
> -  For example, one could compute a graph on 'clone', 'fetch', or 'repack'
> -  commands.

All right; actually it was done by augmenting 'gc' instead.

> -
>  - 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

Good work,
-- 
Jakub Narębski