Re: [PATCH] commit-graph: split up close_reachable() progress output
- From
SZEDER Gábor <szeder.dev@gmail.com>
- Date
- Nov 19, 2018, 22:57 UTC
- Message-ID
- <20181119225720.GV30222@szeder.dev>
- In-Reply-To
- <20181119202300.18670-1-avarab@gmail.com>
On Mon, Nov 19, 2018 at 08:23:00PM +0000, Ævar Arnfjörð Bjarmason wrote:
> Amend the progress output added in 7b0f229222 ("commit-graph write:
> add progress output", 2018-09-17) so that the total numbers it reports
> aren't higher than the total number of commits anymore. See [1] for a
> bug report pointing that out.Please make the commit message more self-contained, i.e. describe the issue this patch fixes in more detail, so readers won't have to follow links to understand the problem.
Show 12 quoted lines
> When I added this I wasn't intending to provide an accurate count, but > just have some progress output to show the user the command wasn't > hanging[2]. But since we are showing numbers, let's make them > accurate. The progress descriptions were suggested by Derrick Stolee > in [3]. > > As noted in [2] we are unlikely to show anything except the "Expanding > reachable..." message even on fairly large repositories such as > linux.git. On a test repository I have with north of 7 million commits > all of these are displayed. Two of them don't show up for long, but as > noted in [5] future-proofing this for if the loops become more > expensive in the future makes sense.
In my opinion this is rather one of those "we'll cross that bridge when (or if ever) we get there" situations.
Show 22 quoted lines
> 1. https://public-inbox.org/git/20181010203738.GE23446@szeder.dev/ > 2. https://public-inbox.org/git/87pnwhea8y.fsf@evledraar.gmail.com/ > 3. https://public-inbox.org/git/f7a0cbee-863c-61d3-4959-5cec8b43c705@gmail.com/ > 4. https://public-inbox.org/git/20181015160545.GG19800@szeder.dev/ > 5. https://public-inbox.org/git/87murle8da.fsf@evledraar.gmail.com/ > > Reported-by: SZEDER Gábor <szeder.dev@gmail.com> > Helped-by: Derrick Stolee <stolee@gmail.com> > Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> > --- > > On Mon, Nov 19 2018, SZEDER Gábor wrote: > > > Ping? > > > > We are at -rc0, this progress output is a new feature since v2.19.0, > > and the numbers shown are still way off. > > I was under the impression after your > https://public-inbox.org/git/20181015160545.GG19800@szeder.dev/ that > you were going to do some more digging & report back, so I put it on > my "waiting for feedback" list and then forgot about it.
No, after I managed to "get my numbers straight" I sent another bug report in
https://public-inbox.org/git/20181015165447.GH19800@szeder.dev/
but as a reply to your original patch. Sorry about the confusion.
> But here's a patch that should address the issue you pointed out, but > I don't know if it fixes whatever you were alluding to in the linked > E-Mail above.
I'm afraid this patch doesn't address that issue, as it's limited to close_reachable(), and that issue is related to the progress output in add_packed_commits().
Show 55 quoted lines
> commit-graph.c | 13 ++++++++++---
> 1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/commit-graph.c b/commit-graph.c
> index 40c855f185..9c0d6914be 100644
> --- a/commit-graph.c
> +++ b/commit-graph.c
> @@ -641,26 +641,29 @@ static void add_missing_parents(struct packed_oid_list *oids, struct commit *com
>
> static void close_reachable(struct packed_oid_list *oids, int report_progress)
> {
> - int i;
> + int i, j;
> struct commit *commit;
> struct progress *progress = NULL;
> - int j = 0;
>
> if (report_progress)
> progress = start_delayed_progress(
> - _("Annotating commits in commit graph"), 0);
> + _("Loading known commits in commit graph"), j = 0);
> for (i = 0; i < oids->nr; i++) {
> display_progress(progress, ++j);
> commit = lookup_commit(the_repository, &oids->list[i]);
> if (commit)
> commit->object.flags |= UNINTERESTING;
> }
> + stop_progress(&progress);
>
> /*
> * As this loop runs, oids->nr may grow, but not more
> * than the number of missing commits in the reachable
> * closure.
> */
> + if (report_progress)
> + progress = start_delayed_progress(
> + _("Expanding reachable commits in commit graph"), j = 0);
> for (i = 0; i < oids->nr; i++) {
> display_progress(progress, ++j);
> commit = lookup_commit(the_repository, &oids->list[i]);
> @@ -668,7 +671,11 @@ static void close_reachable(struct packed_oid_list *oids, int report_progress)
> if (commit && !parse_commit(commit))
> add_missing_parents(oids, commit);
> }
> + stop_progress(&progress);
>
> + if (report_progress)
> + progress = start_delayed_progress(
> + _("Clearing commit marks in commit graph"), j = 0);
> for (i = 0; i < oids->nr; i++) {
> display_progress(progress, ++j);
> commit = lookup_commit(the_repository, &oids->list[i]);
> --
> 2.19.1.1182.g4ecb1133ce
>