Fix --progress flag to properly control progress display and default …#2698
Fix --progress flag to properly control progress display and default …#2698Cyan4973 merged 1 commit intofacebook:devfrom
Conversation
terrelln
left a comment
There was a problem hiding this comment.
This looks good to me, once the build warning is fixed!
I think there is a follow up though, for both compression & decompression:
zstd -c --progress /path/to/file > /dev/null
zstd -dc --progress /path/to/file > /dev/null
In both of these cases, no newline is printed at the end of compression. Normally, the progress is replaced with a status line, like these commands:
zstd /path/to/file -o /dev/null
zstd -d /path/to/file -o /dev/null
I think we should either:
- Add a
\nafter the last progress message. - Also print the final status line with
--progress.
I don't have a super strong preference, but I would go with (2) unless we have an explicit reason why we chose not to.
dfb852e to
60e2072
Compare
programs/fileio.c
Outdated
| double const timeLength_s = (double)timeLength_ns / 1000000000; | ||
| double const cpuLoad_pct = (cpuLoad_s / timeLength_s) * 100; | ||
| DISPLAYLEVEL(4, "%-20s : Completed in %.2f sec (cpu load : %.0f%%)\n", | ||
| DISPLAYLEVEL((g_display_prefs.progressSetting == FIO_ps_always ? 1 : 4), |
There was a problem hiding this comment.
This seems separate from the --progress notification issue.
This message is more part of the final status.
And I don't believe that this additional information (time spent) belongs to default, which should remain restricted to 1 line.
We may have room for discussion in determining if it's more appropriate for level 3 (-v) or 4 (-vv),
but it should certainly not be part of 2 (default).
There was a problem hiding this comment.
This was added per Nick's comment suggesting we add the final status line when using --progress to overwrite the last progress message; it will still remain at level 4 in absence of the progress flag
There was a problem hiding this comment.
The final status line is just the line specifying the final size and compression ratio.
Information about timing and speed are considered supplementary (and do constitute an additional line).
There was a problem hiding this comment.
Ah got it, will update
…progress display on when using -v
…progress display on when using -v