git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: "SZEDER Gábor" <szeder.dev@gmail.com>
Cc: git@vger.kernel.org, "Junio C Hamano" <gitster@pobox.com>,
	"Derrick Stolee" <stolee@gmail.com>, "Jeff King" <peff@peff.net>,
	"Eric Sunshine" <sunshine@sunshineco.com>,
	"Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
Subject: Re: [PATCH v3 1/2] commit-graph write: add progress output
Date: Thu, 11 Oct 2018 00:37:21 +0200	[thread overview]
Message-ID: <87murle8da.fsf@evledraar.gmail.com> (raw)
In-Reply-To: <20181010221951.GG23446@szeder.dev>


On Wed, Oct 10 2018, SZEDER Gábor wrote:

> On Wed, Oct 10, 2018 at 11:56:45PM +0200, Ævar Arnfjörð Bjarmason wrote:
>> On Wed, Oct 10 2018, SZEDER Gábor wrote:
>
>> >>  	for (i = 0; i < oids->nr; i++) {
>> >> +		display_progress(progress, ++j);
>>
>> [...]
>>
>> > This display_progress() call, however, doesn't seem to be necessary.
>> > First, it counts all commits for a second time, resulting in the ~2x
>> > difference compared to the actual number of commits, and then causing
>> > my confusion.  Second, all what this loop is doing is setting a flag
>> > in commits that were already looked up and parsed in the above loops.
>> > IOW this loop is very fast, and the progress indicator jumps from
>> > ~780k right to 1.5M, even on my tiny laptop, so it doesn't need a
>> > progress indicator at all.
>>
>> You're right, I tried this patch on top:
>
> [...]
>
>> And on a large repo with around 3 million commits the 3rd progress bar
>> doesn't kick in.
>>
>> But if I apply this on top:
>>
> [...]
>>
>> I.e. start the timer after 1/4 of a second instead of 1 second, I get
>> that progress bar.
>>
>> So I'm inclined to keep it. It just needs to be 4x the size before it's
>> noticeably hanging for 1 second.
>
> Just to clarify: are you worried about a 1 second hang in an approx. 12
> million commit repository?  If so, then I'm unconvinced, that's not
> even a blip on the radar, and the misleading numbers are far worse.

It's not a blip on the runtime, but the point of these progress bars in
general is so we don't have a UI where there's no UI differnce between
git hanging and just doing work in some tight loop in the background,
and even 1 second when you're watching something is noticeable if it
stalls.

Also it's 1 second on a server where I had 128G of RAM. I think even a
"trivial" flag change like this would very much change if e.g. the
system was under memory pressure or was swapping.

And as noted code like this tends to change over time, that loop might
get more expensive, so let's future proof by having all the loops over N
call the progress code.

When I wrote this the intent was just "report progress". So that it's
counting anything is just an implementation detail of how progress.c
works now.

This was the reference to Duy's patch, i.e. instead of spewing numbers
at the user here let's just render a spinner. Then we no longer need to
make judgement calls about which loop over N is expensive right now, and
which one isn't, and if any of them will result in reporting a 2N number
while the user might be more familiar with or expecting N.

>> That repo isn't all that big compared to what we've heard about out
>> there, and inner loops like this have a tendency to accumulate some more
>> code over time without a re-visit of why we weren't monitoring progress
>> there.
>>
>> But maybe we can fix the message. We say "Annotating commits in commit
>> grap", not "Counting" or whatever. I was trying to find something that
>> didn't imply that we were doing this once. One can annotate a thing more
>> than once, but maybe ther's a better way to explain this...
>
> IMO just remove it.

  reply	other threads:[~2018-10-10 22:37 UTC|newest]

Thread overview: 133+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-04 20:27 [PATCH 0/2] commit-graph: " Æ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
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 [this message]
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=87murle8da.fsf@evledraar.gmail.com \
    --to=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=pclouds@gmail.com \
    --cc=peff@peff.net \
    --cc=stolee@gmail.com \
    --cc=sunshine@sunshineco.com \
    --cc=szeder.dev@gmail.com \
    --subject='Re: [PATCH v3 1/2] commit-graph write: add progress output' \
    /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

Code repositories for project(s) associated with this 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).