git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Derrick Stolee <stolee@gmail.com>
Cc: Junio C Hamano <gitster@pobox.com>, git@vger.kernel.org
Subject: Re: [PATCH 1/2] commit-graph write: add progress output
Date: Fri, 07 Sep 2018 17:23:31 +0200	[thread overview]
Message-ID: <87efe5qqks.fsf@evledraar.gmail.com> (raw)
In-Reply-To: <87ftylqr4z.fsf@evledraar.gmail.com>


On Fri, Sep 07 2018, Ævar Arnfjörð Bjarmason wrote:

> On Wed, Sep 05 2018, Derrick Stolee wrote:
>
>> On 9/4/2018 6:07 PM, Junio C Hamano wrote:
>>> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
>>>
>>>> With --stdin-packs we don't show any estimation of how much is left to
>>>> do. This is because we might be processing more than one pack. We
>>>> could be less lazy here and show progress, either detect by detecting
>>>> that we're only processing one pack, or by first looping over the
>>>> packs to discover how many commits they have. I don't see the point in
>>> I do not know if there is no point, but if we were to do it, I think
>>> slurping the list of packs and computing the number of objects is
>>> not all that bad.
>>
>> If you want to do that, I have nothing against it. However, I don't
>> expect users to use that option directly. That option is used by VFS
>> for Git to compute the commit-graph in the background after receiving
>> a pack of commits and trees, but not by 'git gc' which I expect is how
>> most users will compute commit-graphs.
>>
>>>>   static void compute_generation_numbers(struct packed_commit_list* commits)
>>>>   {
>>>>   	int i;
>>>>   	struct commit_list *list = NULL;
>>>> +	struct progress *progress = NULL;
>>>>   +	progress = start_progress(
>>>> +		_("Computing commit graph generation numbers"), commits->nr);
>>>>   	for (i = 0; i < commits->nr; i++) {
>>>> +		display_progress(progress, i);
>>>>   		if (commits->list[i]->generation != GENERATION_NUMBER_INFINITY &&
>>>>   		    commits->list[i]->generation != GENERATION_NUMBER_ZERO)
>>>>   			continue;
>>> I am wondering if the progress call should be moved after this
>>> conditional continue; would we want to count the entry whose
>>> generation is already known here?  Of course, as we give commits->nr
>>> as the 100% ceiling, we cannot avoid doing so, but it somehow smells
>>> wrong.
>>
>> If we wanted to be completely right, we would count the commits in the
>> list that do not have a generation number and report that as the 100%
>> ceiling.
>>
>> Something like the diff below would work. I tested it in Linux by
>> first deleting my commit-graph and running the following:
>>
>> stolee@stolee-linux:~/linux$ rm .git/objects/info/commit-graph
>> stolee@stolee-linux:~/linux$ git rev-parse v4.6 | ~/git/git
>> commit-graph write --stdin-commits
>> Annotating commits in commit graph: 1180333, done.
>> Computing commit graph generation numbers: 100% (590166/590166), done.
>> stolee@stolee-linux:~/linux$ ~/git/git commit-graph write --reachable
>> Annotating commits in commit graph: 1564087, done.
>> Computing commit graph generation numbers: 100% (191590/191590), done.
>>
>> -->8--
>>
>> From: Derrick Stolee <dstolee@microsoft.com>
>> Date: Wed, 5 Sep 2018 11:55:42 +0000
>> Subject: [PATCH] fixup! commit-graph write: add progress output
>>
>> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
>> ---
>> commit-graph.c | 15 +++++++++++----
>> 1 file changed, 11 insertions(+), 4 deletions(-)
>>
>> diff --git a/commit-graph.c b/commit-graph.c
>> index 1a02fe019a..b933bc9f00 100644
>> --- a/commit-graph.c
>> +++ b/commit-graph.c
>> @@ -634,14 +634,20 @@ static void close_reachable(struct
>> packed_oid_list *oids)
>>
>> static void compute_generation_numbers(struct packed_commit_list* commits)
>> {
>> - int i;
>> + int i, count_uncomputed = 0;
>>  struct commit_list *list = NULL;
>>  struct progress *progress = NULL;
>>
>> + for (i = 0; i < commits->nr; i++)
>> + if (commits->list[i]->generation ==
>> GENERATION_NUMBER_INFINITY ||
>> + commits->list[i]->generation == GENERATION_NUMBER_ZERO)
>> + count_uncomputed++;
>> +
>>  progress = start_progress(
>> - _("Computing commit graph generation numbers"),
>> commits->nr);
>> + _("Computing commit graph generation numbers"),
>> count_uncomputed);
>> + count_uncomputed = 0;
>> +
>>  for (i = 0; i < commits->nr; i++) {
>> - display_progress(progress, i);
>>  if (commits->list[i]->generation !=
>> GENERATION_NUMBER_INFINITY &&
>>  commits->list[i]->generation != GENERATION_NUMBER_ZERO)
>>  continue;
>> @@ -670,10 +676,11 @@ static void compute_generation_numbers(struct
>> packed_commit_list* commits)
>>
>>  if (current->generation >
>> GENERATION_NUMBER_MAX)
>>  current->generation =
>> GENERATION_NUMBER_MAX;
>> +
>> + display_progress(progress,
>> ++count_uncomputed);
>>  }
>>  }
>>  }
>> - display_progress(progress, i);
>>  stop_progress(&progress);
>> }
>
> One of the things I was trying to do with this series was to make sure
> that whenever we run "git gc" there's always some indication that if you
> set gc.writeCommitGraph=true that it's actualy doing work.
>
> This modifies that, which I think is actually fine, just something I
> wanted to note. I.e. if you run "git commit-graph write" twice in a row,
> the second time will have no output.
>
> Unless that is, your repo is big enough that some of the delayed timers
> kick in. So e.g. on git.git we get no output the second time around, but
> do get output the first time around, and on linux.git we always get
> output.
>
> But in the common case people aren't running this in a loop, and it's
> useful to see how many new things are being added to the graph, so I
> think this is better. Just wanted to note the behavior difference (and
> will change the commit message).

Hrm, no. I spoke too soon because I was conflating "commit-graph write"
v.s. "gc". For "gc" we're now with this change just e.g. spending 6
seconds on 2015-04-03-1M-git displaying nothing, because we're looping
through the commits and finding that we have no new work.

So I'm on the fence about this, but leaning towards just taking my
initial approch. I.e. it sucks if you're e.g. testing different "git gc"
options that we're churning in the background doing nothing, just
because we're trying to report how many *new* things we added to the
graph.

After all, the main point IMNSHO is not to show some diagnostic output
of exactly how much work we're doing, that I have 200 new commits with
generation numbers or whatever is just useless trivia, but rather to not
leave the user thinking the command is hanging.

So I think I'll just do what I was doing to begin with and change the
message to "Refreshing commit graph generation numbers" or something to
indicate that it's a find/verify/compute operation, not just a compute
operation.

  reply	other threads:[~2018-09-07 15:23 UTC|newest]

Thread overview: 133+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-04 20:27 [PATCH 0/2] commit-graph: add progress output Ævar Arnfjörð Bjarmason
2018-09-04 20:27 ` [PATCH 1/2] commit-graph write: " Ævar Arnfjörð Bjarmason
2018-09-04 21:16   ` Eric Sunshine
2018-09-04 22:07   ` Junio C Hamano
2018-09-05 11:58     ` Derrick Stolee
2018-09-05 12:07       ` Ævar Arnfjörð Bjarmason
2018-09-05 21:46       ` Junio C Hamano
2018-09-05 22:12         ` Derrick Stolee
2018-09-07 15:11       ` Ævar Arnfjörð Bjarmason
2018-09-07 15:23         ` Ævar Arnfjörð Bjarmason [this message]
2018-09-07 17:15           ` Jeff King
2018-09-07 17:25             ` Derrick Stolee
2018-09-05 12:06   ` Derrick Stolee
2018-09-07 12:40   ` Ævar Arnfjörð Bjarmason
2018-09-07 13:12     ` Derrick Stolee
2018-09-04 20:27 ` [PATCH 2/2] commit-graph verify: " Ævar Arnfjörð Bjarmason
2018-09-04 22:10   ` Junio C Hamano
2018-09-05 12:07 ` [PATCH 0/2] commit-graph: " Derrick Stolee
2018-09-07 18:29 ` [PATCH v2 " Ævar Arnfjörð Bjarmason
2018-09-11 20:26   ` Junio C Hamano
2018-09-07 18:29 ` [PATCH v2 1/2] commit-graph write: " Ævar Arnfjörð Bjarmason
2018-09-21 20:01   ` Derrick Stolee
2018-09-21 21:43     ` Junio C Hamano
2018-09-21 21:57       ` Junio C Hamano
2018-09-07 18:29 ` [PATCH v2 2/2] commit-graph verify: " Ævar Arnfjörð Bjarmason
2018-09-16  6:55   ` Duy Nguyen
2018-09-17 15:33     ` [PATCH v3 0/2] commit-graph: " Ævar Arnfjörð Bjarmason
2018-09-17 15:33     ` [PATCH v3 1/2] commit-graph write: " Ævar Arnfjörð Bjarmason
2018-10-10 20:37       ` SZEDER Gábor
2018-10-10 21:56         ` Ævar Arnfjörð Bjarmason
2018-10-10 22:19           ` SZEDER Gábor
2018-10-10 22:37             ` Ævar Arnfjörð Bjarmason
2018-10-11 17:52               ` Ævar Arnfjörð Bjarmason
2018-10-15 16:05                 ` SZEDER Gábor
2018-10-12  6:09         ` Junio C Hamano
2018-10-12 15:07           ` Ævar Arnfjörð Bjarmason
2018-10-12 15:12             ` Derrick Stolee
2018-10-15 16:54       ` SZEDER Gábor
2018-11-19 16:02         ` SZEDER Gábor
2018-11-19 20:23           ` [PATCH] commit-graph: split up close_reachable() " Ævar Arnfjörð Bjarmason
2018-11-19 20:38             ` Derrick Stolee
2018-11-19 22:57             ` SZEDER Gábor
2018-11-20 15:04               ` [PATCH 0/6] commit-graph write: progress output improvements Ævar Arnfjörð Bjarmason
2018-11-20 15:04               ` [PATCH 1/6] commit-graph write: rephrase confusing progress output Ævar Arnfjörð Bjarmason
2018-11-20 15:04               ` [PATCH 2/6] commit-graph write: add more " Ævar Arnfjörð Bjarmason
2018-11-20 16:58                 ` SZEDER Gábor
2018-11-20 19:50                   ` [PATCH v2 0/6] commit-graph write: progress output improvements Ævar Arnfjörð Bjarmason
2018-11-20 19:50                   ` [PATCH v2 1/6] commit-graph write: rephrase confusing progress output Ævar Arnfjörð Bjarmason
2018-11-20 19:50                   ` [PATCH v2 2/6] commit-graph write: add more " Ævar Arnfjörð Bjarmason
2018-11-20 23:38                     ` SZEDER Gábor
2018-11-20 19:50                   ` [PATCH v2 3/6] commit-graph write: show progress for object search Ævar Arnfjörð Bjarmason
2018-11-20 19:50                   ` [PATCH v2 4/6] commit-graph write: add more describing progress output Ævar Arnfjörð Bjarmason
2018-11-20 19:50                   ` [PATCH v2 5/6] commit-graph write: remove empty line for readability Ævar Arnfjörð Bjarmason
2018-11-20 19:50                   ` [PATCH v2 6/6] commit-graph write: add even more progress output Ævar Arnfjörð Bjarmason
2018-11-21  1:23                   ` SZEDER Gábor
2018-11-21  1:25                     ` [PATCH 1/2] commit-graph: rename 'num_extra_edges' variable to 'num_large_edges' SZEDER Gábor
2018-11-21  3:29                       ` Junio C Hamano
2018-11-21 11:32                         ` Derrick Stolee
2019-01-18 17:05                           ` [PATCH v2 0/2] commit-graph: minor cleanup and optimization SZEDER Gábor
2019-01-18 17:05                             ` [PATCH v2 1/2] commit-graph: rename "large edges" to "extra edges" SZEDER Gábor
2019-01-18 17:05                             ` [PATCH v2 2/2] commit-graph: don't call write_graph_chunk_large_edges() unnecessarily SZEDER Gábor
2019-01-19  9:32                               ` Martin Ågren
2019-01-18 19:41                             ` [PATCH v2 0/2] commit-graph: minor cleanup and optimization Junio C Hamano
2018-11-21  1:26                     ` [PATCH 2/2] commit-graph: don't call write_graph_chunk_large_edges() unnecessarily SZEDER Gábor
2018-11-21 11:33                       ` Derrick Stolee
2018-11-22 13:28                       ` [PATCH v3 00/10] commit-graph write: progress output improvements Ævar Arnfjörð Bjarmason
2018-11-22 15:39                         ` Ævar Arnfjörð Bjarmason
2018-11-22 15:39                         ` [PATCH v4 01/10] commit-graph: rename 'num_extra_edges' variable to 'num_large_edges' Ævar Arnfjörð Bjarmason
2019-01-16 13:29                           ` [PATCH v5 0/9] commit-graph write: progress output improvements Ævar Arnfjörð Bjarmason
2019-01-19  1:26                             ` Junio C Hamano
2019-01-19 20:21                             ` [PATCH v6 00/10] " Ævar Arnfjörð Bjarmason
2019-01-22 18:30                               ` Derrick Stolee
2019-01-23 17:52                                 ` Junio C Hamano
2019-01-22 19:37                               ` Junio C Hamano
2019-01-19 20:21                             ` [PATCH v6 01/10] commit-graph write: use pack order when finding commits Ævar Arnfjörð Bjarmason
2019-01-19 20:21                             ` [PATCH v6 02/10] commit-graph: rename "large edges" to "extra edges" Ævar Arnfjörð Bjarmason
2019-01-19 20:21                             ` [PATCH v6 03/10] commit-graph: don't call write_graph_chunk_large_edges() unnecessarily Ævar Arnfjörð Bjarmason
2019-01-23 17:51                               ` [PATCH v6.1 03/10] commit-graph: don't call write_graph_chunk_extra_edges() unnecessarily SZEDER Gábor
2019-01-19 20:21                             ` [PATCH v6 04/10] commit-graph write: add "Writing out" progress output Ævar Arnfjörð Bjarmason
2019-01-19 20:21                             ` [PATCH v6 05/10] commit-graph write: more descriptive "writing out" output Ævar Arnfjörð Bjarmason
2019-01-19 20:21                             ` [PATCH v6 06/10] commit-graph write: show progress for object search Ævar Arnfjörð Bjarmason
2019-01-19 20:21                             ` [PATCH v6 07/10] commit-graph write: add more descriptive progress output Ævar Arnfjörð Bjarmason
2019-01-19 20:21                             ` [PATCH v6 08/10] commit-graph write: remove empty line for readability Ævar Arnfjörð Bjarmason
2019-01-19 20:21                             ` [PATCH v6 09/10] commit-graph write: add itermediate progress Ævar Arnfjörð Bjarmason
2019-01-19 20:21                             ` [PATCH v6 10/10] commit-graph write: emit a percentage for all progress Ævar Arnfjörð Bjarmason
2019-03-22 10:28                               ` [PATCH] commit-graph: don't show progress percentages while expanding reachable commits SZEDER Gábor
2019-03-22 11:11                                 ` Ævar Arnfjörð Bjarmason
2019-03-22 11:18                                   ` SZEDER Gábor
2019-03-22 14:28                                     ` Ævar Arnfjörð Bjarmason
2019-03-22 14:36                                       ` Ævar Arnfjörð Bjarmason
2019-03-22 14:55                                       ` SZEDER Gábor
2019-03-22 15:11                                         ` Ævar Arnfjörð Bjarmason
2019-03-22 15:49                                           ` SZEDER Gábor
2019-03-22 16:52                                             ` SZEDER Gábor
2019-03-22 17:23                                             ` Ævar Arnfjörð Bjarmason
2019-01-16 13:29                           ` [PATCH v5 1/9] commit-graph: rename 'num_extra_edges' variable to 'num_large_edges' Ævar Arnfjörð Bjarmason
2019-01-16 13:29                           ` [PATCH v5 2/9] commit-graph: don't call write_graph_chunk_large_edges() unnecessarily Ævar Arnfjörð Bjarmason
2019-01-16 13:29                           ` [PATCH v5 3/9] commit-graph write: add "Writing out" progress output Ævar Arnfjörð Bjarmason
2019-01-18 17:16                             ` SZEDER Gábor
2019-01-16 13:29                           ` [PATCH v5 4/9] commit-graph write: more descriptive "writing out" output Ævar Arnfjörð Bjarmason
2019-01-16 13:29                           ` [PATCH v5 5/9] commit-graph write: show progress for object search Ævar Arnfjörð Bjarmason
2019-01-16 13:29                           ` [PATCH v5 6/9] commit-graph write: add more descriptive progress output Ævar Arnfjörð Bjarmason
2019-01-16 13:29                           ` [PATCH v5 7/9] commit-graph write: remove empty line for readability Ævar Arnfjörð Bjarmason
2019-01-16 13:29                           ` [PATCH v5 8/9] commit-graph write: add itermediate progress Ævar Arnfjörð Bjarmason
2019-01-16 13:29                           ` [PATCH v5 9/9] commit-graph write: emit a percentage for all progress Ævar Arnfjörð Bjarmason
2019-01-17 13:23                             ` [PATCH] commit-graph write: use pack order when finding commits Ævar Arnfjörð Bjarmason
2019-01-17 15:09                               ` Derrick Stolee
2019-01-17 16:35                                 ` Derrick Stolee
2018-11-22 15:39                         ` [PATCH v4 02/10] commit-graph: don't call write_graph_chunk_large_edges() unnecessarily Ævar Arnfjörð Bjarmason
2018-11-22 15:39                         ` [PATCH v4 03/10] commit-graph write: rephrase confusing progress output Ævar Arnfjörð Bjarmason
2018-11-22 15:39                         ` [PATCH v4 04/10] commit-graph write: add "Writing out" " Ævar Arnfjörð Bjarmason
2018-11-22 15:39                         ` [PATCH v4 05/10] commit-graph write: more descriptive "writing out" output Ævar Arnfjörð Bjarmason
2018-11-22 15:39                         ` [PATCH v4 06/10] commit-graph write: show progress for object search Ævar Arnfjörð Bjarmason
2018-11-22 15:39                         ` [PATCH v4 07/10] commit-graph write: add more descriptive progress output Ævar Arnfjörð Bjarmason
2018-11-22 15:39                         ` [PATCH v4 08/10] commit-graph write: remove empty line for readability Ævar Arnfjörð Bjarmason
2018-11-22 15:39                         ` [PATCH v4 09/10] commit-graph write: add itermediate progress Ævar Arnfjörð Bjarmason
2018-11-22 15:39                         ` [PATCH v4 10/10] commit-graph write: emit a percentage for all progress Ævar Arnfjörð Bjarmason
2018-11-22 18:59                         ` [PATCH v3 00/10] commit-graph write: progress output improvements Eric Sunshine
2018-11-22 13:28                       ` [PATCH v3 01/10] commit-graph: rename 'num_extra_edges' variable to 'num_large_edges' Ævar Arnfjörð Bjarmason
2018-11-22 13:28                       ` [PATCH v3 02/10] commit-graph: don't call write_graph_chunk_large_edges() unnecessarily Ævar Arnfjörð Bjarmason
2018-11-22 13:28                       ` [PATCH v3 03/10] commit-graph write: rephrase confusing progress output Ævar Arnfjörð Bjarmason
2018-11-22 13:28                       ` [PATCH v3 04/10] commit-graph write: add "Writing out" " Ævar Arnfjörð Bjarmason
2018-11-22 13:28                       ` [PATCH v3 05/10] commit-graph write: more descriptive "writing out" output Ævar Arnfjörð Bjarmason
2018-11-22 13:28                       ` [PATCH v3 06/10] commit-graph write: show progress for object search Ævar Arnfjörð Bjarmason
2018-11-22 13:28                       ` [PATCH v3 07/10] commit-graph write: add more descriptive progress output Ævar Arnfjörð Bjarmason
2018-11-22 13:28                       ` [PATCH v3 08/10] commit-graph write: remove empty line for readability Ævar Arnfjörð Bjarmason
2018-11-22 13:28                       ` [PATCH v3 09/10] commit-graph write: add itermediate progress Ævar Arnfjörð Bjarmason
2018-11-22 13:28                       ` [PATCH v3 10/10] commit-graph write: emit a percentage for all progress Ævar Arnfjörð Bjarmason
2018-11-20 15:04               ` [PATCH 3/6] commit-graph write: show progress for object search Ævar Arnfjörð Bjarmason
2018-11-20 15:04               ` [PATCH 4/6] commit-graph write: add more describing progress output Ævar Arnfjörð Bjarmason
2018-11-20 15:04               ` [PATCH 5/6] commit-graph write: remove empty line for readability Ævar Arnfjörð Bjarmason
2018-11-20 15:04               ` [PATCH 6/6] commit-graph write: add even more progress output Ævar Arnfjörð Bjarmason
2018-09-17 15:33     ` [PATCH v3 2/2] commit-graph verify: add " Ævar Arnfjörð Bjarmason

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87efe5qqks.fsf@evledraar.gmail.com \
    --to=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=stolee@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).