Lasse Collin wrote:

> I thought it would be useful to see how long it had been working 
> especially when it fails due to corrupt input when decompressing.

I guess this is to know how long it would take to reproduce?

> Showing both estimate 
> and remaining time would need reworking the progress message line so 
> that all the info would fit into 80 chars.

  100.0 %     10,000,000 MiB / 10,000,000 MiB = 1.000    999 KiB/s   9 h 40 min
(current)

  100.0 %     10,000,000 MiB / 10,000,000 MiB = 1.000    9999:50:50  9 h 40 min
(elapsed time replaces compression speed on completion)

  100.0 %   10,000 GiB / 10,000 GiB = 1.000   999 KiB/s   9 h 40 min  9999:50:50
(less padding, more units)

I kind of like #2.

Speeds in progress displays can be ambiguous, anyway: they could
represent the average speed over the whole period, including long
stalls from e.g. suspending the process (as xz’s does) or they can be a
running average over a representative interval of recent time.  I think
of them as more meaningful during the compression.

> The patch won't work correctly, because progress_active would be left to 
> true.

Yes, sorry.  It was only good enough to demonstrate what this would
look like when compressing a single file.

How about something like this instead?

-- %< --
Subject: xz -v: Leave time remaining on screen on failure

 - When timing xz, it is nice to have an estimate of how much time
   was left before hitting ^C and giving up.

 - When debugging with corrupted input, it is nice to know about
   how long it will take to hit the same corruption on a second
   run.

Output both statistics on the progress line after a failed run.

The previous output only used the time elapsed, which was confusing
because without the numbers changing there is no cue to indicate
whether they are supposed to go up or down with time.

Now the time elapsed replaces the speed on the final progress line.
To make room, the counts of input processed / output produced lose
some precision for counts of 10.0 GiB or more.

Requested-by: Trent W. Buck <[email protected]>
Signed-off-by: Jonathan Nieder <[email protected]>
---
 src/xz/message.c |   30 +++++++++++++++++-------------
 1 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/src/xz/message.c b/src/xz/message.c
index 5dd9bc3..b535a33 100644
--- a/src/xz/message.c
+++ b/src/xz/message.c
@@ -343,11 +343,11 @@ progress_percentage(uint64_t in_pos, bool final)
 static const char *
 progress_sizes(uint64_t compressed_pos, uint64_t uncompressed_pos, bool final)
 {
-       // This is enough to hold sizes up to about 99 TiB if thousand
-       // separator is used, or about 1 PiB without thousand separator.
+       // This is enough to hold sizes up to about 9 PiB if thousand
+       // separator is used, or about 900 PiB without thousand separator.
        // After that the progress indicator will look a bit silly, since
        // the compression ratio no longer fits with three decimal places.
-       static char buf[44];
+       static char buf[43];
 
        char *pos = buf;
        size_t left = sizeof(buf);
@@ -357,9 +357,9 @@ progress_sizes(uint64_t compressed_pos, uint64_t 
uncompressed_pos, bool final)
        const enum nicestr_unit unit_min = final ? NICESTR_B : NICESTR_MIB;
        my_snprintf(&pos, &left, "%s / %s",
                        uint64_to_nicestr(compressed_pos,
-                               unit_min, NICESTR_MIB, false, 0),
+                               unit_min, NICESTR_GIB, false, 0),
                        uint64_to_nicestr(uncompressed_pos,
-                               unit_min, NICESTR_MIB, false, 1));
+                               unit_min, NICESTR_GIB, false, 1));
 
        // Avoid division by zero. If we cannot calculate the ratio, set
        // it to some nice number greater than 10.0 so that it gets caught
@@ -454,7 +454,7 @@ progress_time(uint64_t useconds)
 /// Make the string to contain the estimated remaining time, or if the amount
 /// of input isn't known, how much time has elapsed.
 static const char *
-progress_remaining(uint64_t in_pos, uint64_t elapsed)
+progress_remaining(uint64_t in_pos, uint64_t elapsed, bool final)
 {
        // Show the amount of time spent so far when making an estimate of
        // remaining time wouldn't be reasonable:
@@ -464,8 +464,8 @@ progress_remaining(uint64_t in_pos, uint64_t elapsed)
        //    too inaccurate.
        //  - Only a few seconds has passed since we started (de)compressing,
        //    so estimate would be too inaccurate.
-       if (expected_in_size == 0 || in_pos > expected_in_size
-                       || in_pos < (UINT64_C(1) << 19) || elapsed < 8000000)
+       if (!final && (expected_in_size == 0 || in_pos > expected_in_size
+                       || in_pos < (UINT64_C(1) << 19) || elapsed < 8000000))
                return progress_time(elapsed);
 
        // Calculate the estimate. Don't give an estimate of zero seconds,
@@ -599,11 +599,11 @@ message_progress_update(void)
        // Print the actual progress message. The idea is that there is at
        // least three spaces between the fields in typical situations, but
        // even in rare situations there is at least one space.
-       fprintf(stderr, "  %7s %43s   %9s   %10s\r",
+       fprintf(stderr, "  %7s %42s   %10s   %10s\r",
                progress_percentage(in_pos, false),
                progress_sizes(compressed_pos, uncompressed_pos, false),
                progress_speed(uncompressed_pos, elapsed),
-               progress_remaining(in_pos, elapsed));
+               progress_remaining(in_pos, elapsed, false));
 
 #ifdef SIGALRM
        // Updating the progress info was finished. Reset
@@ -674,14 +674,18 @@ progress_flush(bool finished)
        // statistics are printed in the same format as the progress
        // indicator itself.
        if (progress_automatic) {
+               // The time remaining isn't always shown.
+               const char *time_remaining = finished ?
+                       "" : progress_remaining(in_pos, elapsed, true);
+
                // Using floating point conversion for the percentage instead
                // of static "100.0 %" string, because the decimal separator
                // isn't a dot in all locales.
-               fprintf(stderr, "  %7s %43s   %9s   %10s\n",
+               fprintf(stderr, "  %7s %42s   %10s   %10s\n",
                        progress_percentage(in_pos, finished),
                        progress_sizes(compressed_pos, uncompressed_pos, true),
-                       progress_speed(uncompressed_pos, elapsed),
-                       elapsed_str);
+                       elapsed_str,
+                       time_remaining);
        } else {
                // The filename is always printed.
                fprintf(stderr, "%s: ", filename);
-- 
1.7.0.2




-- 
To UNSUBSCRIBE, email to [email protected]
with a subject of "unsubscribe". Trouble? Contact [email protected]

Reply via email to