Skip to content

Make the CLI summary data use a human readable format instead of raw bytes #2696

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 15 commits into from
Jun 8, 2021
Merged

Make the CLI summary data use a human readable format instead of raw bytes #2696

Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
49dd77b
Make the CLI output the file sizes in human readable format
scottchiefbaker Jun 5, 2021
e93fdd3
Put the human_size() function in util.c
scottchiefbaker Jun 5, 2021
4792532
Convert names to CamelCase
scottchiefbaker Jun 5, 2021
9da459f
Make the variable types match
scottchiefbaker Jun 5, 2021
2342122
Move the variable declarations to the top
scottchiefbaker Jun 5, 2021
e4e7358
Use human_size() in the benchmark output also
scottchiefbaker Jun 5, 2021
dcfaf26
Use human_size() on the "multiple files compressed" output also
scottchiefbaker Jun 5, 2021
b63b497
Convert tabs to spaces
scottchiefbaker Jun 5, 2021
dbb3cd2
human_size() should use size_t
scottchiefbaker Jun 5, 2021
91840eb
Use unsigned long instead to help with some tests
scottchiefbaker Jun 5, 2021
d67d965
Update humanSize() to skip the big numbers (it requires 64 bit)
scottchiefbaker Jun 5, 2021
fdcfc18
Try unsigned long long
scottchiefbaker Jun 5, 2021
a1d128a
Try enabling the BIG strings now the unsigned long long is in effect
scottchiefbaker Jun 5, 2021
10748cb
Some fixes to address things @felixhandte found
scottchiefbaker Jun 7, 2021
23b6c67
Attempt to fix a failing test with help from @aqrit
scottchiefbaker Jun 7, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 17 additions & 5 deletions programs/benchzstd.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -388,6 +388,8 @@ BMK_benchMemAdvancedNoAlloc(
# define NB_MARKS 4
const char* marks[NB_MARKS] = { " |", " /", " =", " \\" };
U32 markNb = 0;
char inputSizeStr[8] = "";
char outputSizeStr[8] = "";
int compressionCompleted = (adv->mode == BMK_decodeOnly);
int decompressionCompleted = (adv->mode == BMK_compressOnly);
BMK_benchParams_t cbp, dbp;
Expand Down Expand Up @@ -429,8 +431,10 @@ BMK_benchMemAdvancedNoAlloc(
dctxprep.dictBuffer = dictBuffer;
dctxprep.dictBufferSize = dictBufferSize;

humanSize((unsigned)srcSize, inputSizeStr);

DISPLAYLEVEL(2, "\r%70s\r", ""); /* blank line */
DISPLAYLEVEL(2, "%2s-%-17.17s :%10u ->\r", marks[markNb], displayName, (unsigned)srcSize);
DISPLAYLEVEL(2, "%2s-%-17.17s : %s -> \r", marks[markNb], displayName, inputSizeStr);

while (!(compressionCompleted && decompressionCompleted)) {
if (!compressionCompleted) {
Expand All @@ -451,9 +455,13 @@ BMK_benchMemAdvancedNoAlloc(
} }

{ int const ratioAccuracy = (ratio < 10.) ? 3 : 2;
DISPLAYLEVEL(2, "%2s-%-17.17s :%10u ->%10u (%5.*f),%6.*f MB/s\r",

humanSize((unsigned)srcSize, inputSizeStr);
humanSize((unsigned)cSize, outputSizeStr);

DISPLAYLEVEL(2, "%2s-%-17.17s : %s -> %s (%5.*f),%6.*f MB/s\r",
marks[markNb], displayName,
(unsigned)srcSize, (unsigned)cSize,
inputSizeStr, outputSizeStr,
ratioAccuracy, ratio,
benchResult.cSpeed < (10 MB) ? 2 : 1, (double)benchResult.cSpeed / MB_UNIT);
}
Expand All @@ -474,9 +482,13 @@ BMK_benchMemAdvancedNoAlloc(
}

{ int const ratioAccuracy = (ratio < 10.) ? 3 : 2;
DISPLAYLEVEL(2, "%2s-%-17.17s :%10u ->%10u (%5.*f),%6.*f MB/s ,%6.1f MB/s \r",

humanSize((unsigned)srcSize, inputSizeStr);
humanSize((unsigned)cSize, outputSizeStr);

DISPLAYLEVEL(2, "%2s-%-17.17s : %s -> %s (%5.*f),%6.*f MB/s ,%6.1f MB/s \r",
marks[markNb], displayName,
(unsigned)srcSize, (unsigned)cSize,
inputSizeStr, outputSizeStr,
ratioAccuracy, ratio,
benchResult.cSpeed < (10 MB) ? 2 : 1, (double)benchResult.cSpeed / MB_UNIT,
(double)benchResult.dSpeed / MB_UNIT);
Expand Down
19 changes: 15 additions & 4 deletions programs/fileio.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1544,6 +1544,9 @@ FIO_compressFilename_internal(FIO_ctx_t* const fCtx,
U64 readsize = 0;
U64 compressedfilesize = 0;
U64 const fileSize = UTIL_getFileSize(srcFileName);
char inputSizeStr[8] = "";
char outputSizeStr[8] = "";

DISPLAYLEVEL(5, "%s: %llu bytes \n", srcFileName, (unsigned long long)fileSize);

/* compression format selection */
Expand Down Expand Up @@ -1598,10 +1601,13 @@ FIO_compressFilename_internal(FIO_ctx_t* const fCtx,
(unsigned long long)readsize, (unsigned long long) compressedfilesize,
dstFileName);
} else {
DISPLAYLEVEL(2,"%-20s :%6.2f%% (%6llu => %6llu bytes, %s) \n",
humanSize((unsigned long long) readsize, inputSizeStr);
humanSize((unsigned long long) compressedfilesize, outputSizeStr);

DISPLAYLEVEL(2,"%-20s :%6.2f%% (%s => %s, %s) \n",
srcFileName,
(double)compressedfilesize / (double)readsize * 100,
(unsigned long long)readsize, (unsigned long long) compressedfilesize,
inputSizeStr, outputSizeStr,
dstFileName);
}
}
Expand Down Expand Up @@ -1835,6 +1841,8 @@ int FIO_compressMultipleFilenames(FIO_ctx_t* const fCtx,
{
int status;
int error = 0;
char inputSizeStr[8] = "";
char outputSizeStr[8] = "";
cRess_t ress = FIO_createCResources(prefs, dictFileName,
FIO_getLargestFileSize(inFileNamesTable, (unsigned)fCtx->nbFilesTotal),
compressionLevel, comprParams);
Expand Down Expand Up @@ -1890,10 +1898,13 @@ int FIO_compressMultipleFilenames(FIO_ctx_t* const fCtx,
}

if (fCtx->nbFilesProcessed >= 1 && fCtx->nbFilesTotal > 1 && fCtx->totalBytesInput != 0) {
humanSize((unsigned long long) fCtx->totalBytesInput, inputSizeStr);
humanSize((unsigned long long) fCtx->totalBytesOutput, outputSizeStr);

DISPLAYLEVEL(2, "\r%79s\r", "");
DISPLAYLEVEL(2, "%d files compressed : %.2f%% (%6zu => %6zu bytes)\n", fCtx->nbFilesProcessed,
DISPLAYLEVEL(2, "%3d files compressed : %.2f%% (%s => %s bytes)\n", fCtx->nbFilesProcessed,
(double)fCtx->totalBytesOutput/((double)fCtx->totalBytesInput)*100,
fCtx->totalBytesInput, fCtx->totalBytesOutput);
inputSizeStr, outputSizeStr);
}

FIO_freeCResources(&ress);
Expand Down
21 changes: 21 additions & 0 deletions programs/util.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,27 @@ int UTIL_requireUserConfirmation(const char* prompt, const char* abortMsg,
* Functions
***************************************/

char* humanSize(unsigned long long size, char* str) {
if (size > 1125899906842624L) {
snprintf(str, 7, "%.1fP", (float)size / 1125899906842624L);
Copy link
Contributor

@felixhandte felixhandte Jun 7, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

snprintf was standardized in C99, so it's not guaranteed to be available in all the environments we want Zstd to build.

You could consider rewriting this to not require an intermediate buffer if you just output a rescaled float value and a (static) suffix string that are then passed to the DISPLAYLEVEL() call with "%.1f%s". But maybe that would lead to poor results for small inputs ("you compressed 123.0B" feels weird).

A worst case alternative is that you just wrap this in an #if POSIX_PLATFORM_VERSION >= 200112L and provide a fallback #else implementation.

Copy link
Contributor Author

@scottchiefbaker scottchiefbaker Jun 7, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see references to fprintf()all over the code base so I just assumed snprintf() was valid. util.c. includes <stdio.h> for fprintf(), can we rely on that, but not snprintf()?

C is not my forte, so I may need some guidance on this one.

Copy link
Contributor

@felixhandte felixhandte Jun 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As C has evolved, they've introduced new functions into the same header. C90 has fprintf() but not necessarily snprintf(). snprintf() was only standardized (and guaranteed to be present) in C99. Zstd restricts itself to the features guaranteed to be present in C90 (or at least has fallbacks to handle that strict case), so that we can be sure it will compile on all C90-conforming platforms.

} else if (size > 1099511627776L) {
snprintf(str, 7, "%.1fT", (float)size / 1099511627776L);
} else if (size > 1073741824L) {
snprintf(str, 7, "%.1fG", (float)size / 1073741824L);
} else if (size > 1048576L) {
snprintf(str, 7, "%.1fM", (float)size / 1048576L);
} else if (size > 1024) {
snprintf(str, 7, "%.1fK", (float)size / 1024);
} else if (size <= 1024) {
snprintf(str, 7, "%lluB", size);
} else {
str[0] = '\0';
}

return str;
}


int UTIL_stat(const char* filename, stat_t* statbuf)
{
#if defined(_MSC_VER)
Expand Down
2 changes: 2 additions & 0 deletions programs/util.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,8 @@ int UTIL_requireUserConfirmation(const char* prompt, const char* abortMsg, const
#define STRDUP(s) strdup(s)
#endif

char* humanSize(unsigned long long size, char* str);
Copy link
Contributor

@felixhandte felixhandte Jun 7, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Switch to a more descriptive name? Maybe add a little comment here describing what it does?

Copy link
Contributor Author

@scottchiefbaker scottchiefbaker Jun 7, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm open to naming suggestions. I will definitely add a description though, that's easy.

Copy link
Contributor

@felixhandte felixhandte Jun 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe UTIL_renderHumanReadableByteSize()?


/**
* Calls platform's equivalent of stat() on filename and writes info to statbuf.
* Returns success (1) or failure (0).
Expand Down