git/progress.c, branch v2.16.2

Merge branch 'en/rename-progress'

2017-12-19T19:33:55Z

Historically, the diff machinery for rename detection had a hardcoded limit of 32k paths; this is being lifted to allow users trade cycles with a (possibly) easier to read result. * en/rename-progress: diffcore-rename: make diff-tree -l0 mean -l sequencer: show rename progress during cherry picks diff: remove silent clamp of renameLimit progress: fix progress meters when dealing with lots of work sequencer: warn when internal merge may be suboptimal due to renameLimit

progress: drop delay-threshold code

2017-12-04T22:22:18Z

Since 180a9f2268 (provide a facility for "delayed" progress reporting, 2007-04-20), the progress code has allowed callers to skip showing progress if they have reached a percentage-threshold of the total work before the delay period passes. But since 8aade107dd (progress: simplify "delayed" progress API, 2017-08-19), that parameter is not available to outside callers (we always passed zero after that commit, though that was corrected in the previous commit to "100%"). Let's drop the threshold code, which never triggers in any meaningful way. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

progress: set default delay threshold to 100%, not 0%

2017-12-04T22:22:17Z

Commit 8aade107dd (progress: simplify "delayed" progress API, 2017-08-19) dropped the parameter by which callers could say "show my progress only if I haven't passed M% progress after N seconds". The intent was to just show nothing for 2 seconds, and then always progress after that. But we flipped the logic in the wrapper: it sets M=0, meaning that we'd almost _never_ show progress after 2 seconds, since we'd generally have made some progress. This should have been 100%, not 0%. We were fooled by existing calls like: start_progress_delay("foo", 0, 0, 2); which behaved this way. The trick is that the first "0" there is "how many items total", and there zero means "we don't know". And without knowing that, we cannot compute a completed percent at all, and we ignored the threshold parameter entirely! Modeling our wrapper after that broke callers which pass a non-zero value for "total". We can switch to the intended behavior by using "100" in the wrapper call. Reported-by: Lars Schneider Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

progress: fix progress meters when dealing with lots of work

2017-11-15T04:11:25Z

The possibility of setting merge.renameLimit beyond 2^16 raises the possibility that the values passed to progress can exceed 2^32. Use uint64_t, because it "ought to be enough for anybody". :-) Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano

progress: simplify "delayed" progress API

2017-08-19T21:01:34Z

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 Signed-off-by: Junio C Hamano

progress: show overall rate in last update

2017-07-09T16:54:20Z

The values in struct throughput are only updated every 0.5 seconds. If we're all done before that time span then the final update will show a rate of 0 bytes/s, which is misleading if some bytes had been handled. Remember the start time and show the total throughput instead. And avoid division by zero by enforcing a minimum time span value of 1 (unit: 1/1024th of a second). That makes the resulting rate an underestimation, but it's closer to the actual value than the currently shown 0 bytes/s. Reported-by: 積丹尼 Dan Jacobson Signed-off-by: Rene Scharfe Signed-off-by: Junio C Hamano

stop_progress_msg: convert xsnprintf to xstrfmt

2017-02-16T19:28:02Z

Simplify code by replacing buffer allocation with a call to xstrfmt(). Signed-off-by: Maxim Moseychuk Signed-off-by: Junio C Hamano

use xmallocz to avoid size arithmetic

2016-02-22T22:51:09Z

We frequently allocate strings as xmalloc(len + 1), where the extra 1 is for the NUL terminator. This can be done more simply with xmallocz, which also checks for integer overflow. There's no case where switching xmalloc(n+1) to xmallocz(n) is wrong; the result is the same length, and malloc made no guarantees about what was in the buffer anyway. But in some cases, we can stop manually placing NUL at the end of the allocated buffer. But that's only safe if it's clear that the contents will always fill the buffer. In each case where this patch does so, I manually examined the control flow, and I tried to err on the side of caution. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

stop_progress_msg: convert sprintf to xsnprintf

2015-09-25T17:18:18Z

The usual arguments for using xsnprintf over sprintf apply, but this case is a little tricky. We print to a fixed-size buffer if we have room, and otherwise to an allocated buffer. So there should be no overflow here, but it is still good to communicate our intention, as well as to check our earlier math for how much space the string will need. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

progress: store throughput display in a strbuf

2015-09-25T17:18:18Z

Coverity noticed that we strncpy() into a fixed-size buffer without making sure that it actually ended up NUL-terminated. This is unlikely to be a bug in practice, since throughput strings rarely hit 32 characters, but it would be nice to clean it up. The most obvious way to do so is to add a NUL-terminator. But instead, this patch switches the fixed-size buffer out for a strbuf. At first glance this seems much less efficient, until we realize that filling in the fixed-size buffer is done by writing into a strbuf and copying the result! By writing straight to the buffer, we actually end up more efficient: 1. We avoid an extra copy of the bytes. 2. Rather than malloc/free each time progress is shown, we can strbuf_reset and use the same buffer each time. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano