@matheusbsbs / matheus-bagliano / commits / 8aade107dd

progress: simplify "delayed" progress API

We used to expose the full power of the delayed progress API to the callers, so that they can specify, not just the message to show and expected total amount of work that is used to compute the percentage of work performed so far, the percent-threshold parameter P and the delay-seconds parameter N. The progress meter starts to show at N seconds into the operation only if we have not yet completed P per-cent of the total work. Most callers used either (0%, 2s) or (50%, 1s) as (P, N), but there are oddballs that chose more random-looking values like 95%. For a smoother workload, (50%, 1s) would allow us to start showing the progress meter earlier than (0%, 2s), while keeping the chance of not showing progress meter for long running operation the same as the latter. For a task that would take 2s or more to complete, it is likely that less than half of it would complete within the first second, if the workload is smooth. But for a spiky workload whose earlier part is easier, such a setting is likely to fail to show the progress meter entirely and (0%, 2s) is more appropriate. But that is merely a theory. Realistically, it is of dubious value to ask each codepath to carefully consider smoothness of their workload and specify their own setting by passing two extra parameters. Let's simplify the API by dropping both parameters and have everybody use (0%, 2s). Oh, by the way, the percent-threshold parameter and the structure member were consistently misspelled, which also is now fixed ;-) Helped-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Junio C Hamano committed Aug 19, 2017 at 10:39 UTC 8aade107dd84dcaff3f23caae80a013878db2de7

9 files changed +19 -18

builtin/blame.c

+1 -2

@@ -925,8 +925,7 @@ parse_done:

925 sb.found_guilty_entry = &found_guilty_entry;

926 sb.found_guilty_entry_data = π

927 if (show_progress)

928 - pi.progress = start_progress_delay(_("Blaming lines"),

929 - sb.num_lines, 50, 1);

928 + pi.progress = start_delayed_progress(_("Blaming lines"), sb.num_lines);

929

930 assign_blame(&sb, opt);

931

builtin/fsck.c

+1 -1

@@ -188,7 +188,7 @@ static int traverse_reachable(void)

188 unsigned int nr = 0;

189 int result = 0;

190 if (show_progress)

191 - progress = start_progress_delay(_("Checking connectivity"), 0, 0, 2);

191 + progress = start_delayed_progress(_("Checking connectivity"), 0);

192 while (pending.nr) {

193 struct object_array_entry *entry;

194 struct object *obj;

builtin/prune-packed.c

+1 -2

@@ -37,8 +37,7 @@ static int prune_object(const struct object_id *oid, const char *path,

37 void prune_packed_objects(int opts)

38 {

39 if (opts & PRUNE_PACKED_VERBOSE)

40 - progress = start_progress_delay(_("Removing duplicate objects"),

41 - 256, 95, 2);

40 + progress = start_delayed_progress(_("Removing duplicate objects"), 256);

42 for_each_loose_file_in_objdir(get_object_directory(),

43 prune_object, NULL, prune_subdir, &opts);

builtin/prune.c

+1 -1

@@ -138,7 +138,7 @@ int cmd_prune(int argc, const char **argv, const char *prefix)

138 if (show_progress == -1)

139 show_progress = isatty(2);

140 if (show_progress)

141 - progress = start_progress_delay(_("Checking connectivity"), 0, 0, 2);

141 + progress = start_delayed_progress(_("Checking connectivity"), 0);

142

143 mark_reachable_objects(&revs, 1, expire, progress);

144 stop_progress(&progress);

builtin/rev-list.c

+1 -1

@@ -364,7 +364,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)

364 revs.limited = 1;

365

366 if (show_progress)

367 - progress = start_progress_delay(show_progress, 0, 0, 2);

367 + progress = start_delayed_progress(show_progress, 0);

368

369 if (use_bitmap_index && !revs.prune) {

370 if (revs.count && !revs.left_right && !revs.cherry_mark) {

diffcore-rename.c

+2 -2

@@ -532,9 +532,9 @@ void diffcore_rename(struct diff_options *options)

532 }

533

534 if (options->show_rename_progress) {

535 - progress = start_progress_delay(

535 + progress = start_delayed_progress(

536 _("Performing inexact rename detection"),

537 - rename_dst_nr * rename_src_nr, 50, 1);

537 + rename_dst_nr * rename_src_nr);

538 }

539

540 mx = xcalloc(st_mult(NUM_CANDIDATE_PER_DST, num_create), sizeof(*mx));

progress.c

+10 -5

@@ -34,7 +34,7 @@ struct progress {

34 unsigned total;

35 unsigned last_percent;

36 unsigned delay;

37 - unsigned delayed_percent_treshold;

37 + unsigned delayed_percent_threshold;

38 struct throughput *throughput;

39 uint64_t start_ns;

40 };

@@ -88,7 +88,7 @@ static int display(struct progress *progress, unsigned n, const char *done)

88 return 0;

89 if (progress->total) {

90 unsigned percent = n * 100 / progress->total;

91 - if (percent > progress->delayed_percent_treshold) {

91 + if (percent > progress->delayed_percent_threshold) {

92 /* inhibit this progress report entirely */

93 clear_progress_signal();

94 progress->delay = -1;

@@ -205,8 +205,8 @@ int display_progress(struct progress *progress, unsigned n)

205 return progress ? display(progress, n, NULL) : 0;

206 }

207

208 -struct progress *start_progress_delay(const char *title, unsigned total,

209 - unsigned percent_treshold, unsigned delay)

208 +static struct progress *start_progress_delay(const char *title, unsigned total,

209 + unsigned percent_threshold, unsigned delay)

210 {

211 struct progress *progress = malloc(sizeof(*progress));

212 if (!progress) {

@@ -219,7 +219,7 @@ struct progress *start_progress_delay(const char *title, unsigned total,

219 progress->total = total;

220 progress->last_value = -1;

221 progress->last_percent = -1;

222 - progress->delayed_percent_treshold = percent_treshold;

222 + progress->delayed_percent_threshold = percent_threshold;

223 progress->delay = delay;

224 progress->throughput = NULL;

225 progress->start_ns = getnanotime();

@@ -227,6 +227,11 @@ struct progress *start_progress_delay(const char *title, unsigned total,

227 return progress;

228 }

229

230 +struct progress *start_delayed_progress(const char *title, unsigned total)

231 +{

232 + return start_progress_delay(title, total, 0, 2);

233 +}

234 +

235 struct progress *start_progress(const char *title, unsigned total)

236 {

237 return start_progress_delay(title, total, 0, 0);

progress.h

+1 -2

@@ -6,8 +6,7 @@ struct progress;

6 void display_throughput(struct progress *progress, off_t total);

7 int display_progress(struct progress *progress, unsigned n);

8 struct progress *start_progress(const char *title, unsigned total);

9 -struct progress *start_progress_delay(const char *title, unsigned total,

10 - unsigned percent_treshold, unsigned delay);

9 +struct progress *start_delayed_progress(const char *title, unsigned total);

10 void stop_progress(struct progress **progress);

11 void stop_progress_msg(struct progress **progress, const char *msg);

unpack-trees.c

+1 -2

@@ -343,8 +343,7 @@ static struct progress *get_progress(struct unpack_trees_options *o)

343 total++;

344 }

345

346 - return start_progress_delay(_("Checking out files"),

347 - total, 50, 1);

346 + return start_delayed_progress(_("Checking out files"), total);

347 }

348

349 static int check_updates(struct unpack_trees_options *o)