Skip to content
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.

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

🐞 zstd may not be using proper compression levels #112

Closed
pete4abw opened this issue Mar 21, 2023 · 3 comments
Closed

🐞 zstd may not be using proper compression levels #112

pete4abw opened this issue Mar 21, 2023 · 3 comments
Assignees
Labels

Comments

@pete4abw
Copy link
Owner

lrzip-next Version

0.10.4

lrzip-next command line

lrzip-next -Z ...

What happened?

I may have misread zstd.h where enumerations for Compression Strategy from 1-9 may have been misinterpreted for Compression Level. Thus, the lrzip-next compression level, 1-9, is passed to ZSTD_compress, but zstd permits compression levels from 1-22. Each of those use a compression strategy! These enumerations are selected from a table based on compression level.

	/* ZSTD compression methods are enumerated from 1 to 9
	 * similar to lrzip-next compression evels. Enumerations are:
	 * ZSTD_fast=1,
	 * ZSTD_dfast=2,
	 * ZSTD_greedy=3,
	 * ZSTD_lazy=4,
	 * ZSTD_lazy2=5,
	 * ZSTD_btlazy2=6,
	 * ZSTD_btopt=7,
	 * ZSTD_btultra=8,
	 * ZSTD_btultra2=9
	 */

What was expected behavior?

zstd compression levels would be set correctly between 1-22 or at least 1-19 expanded from lrzip-next compression level.

Steps to reproduce

See code in stream.c where compression strategies are interpreted (possibly) incorrectly.

 193         /* ZSTD compression methods are enumerated from 1 to 9
 194          * similar to lrzip-next compression evels. Enumerations are:
 195          * ZSTD_fast=1,
 196          * ZSTD_dfast=2,
 197          * ZSTD_greedy=3,
 198          * ZSTD_lazy=4,
 199          * ZSTD_lazy2=5,
 200          * ZSTD_btlazy2=6,
 201          * ZSTD_btopt=7,
 202          * ZSTD_btultra=8,
 203          * ZSTD_btultra2=9
 204          */
 205
 206         print_maxverbose("Starting zstd backend compression thread %d... \n", current_thread);
 207
 208         zstd_ret = ZSTD_compress( (void *)c_buf, (size_t) dlen,
 209                               (const void *)cthread->s_buf, (size_t) cthread->s_len,
 210                               control->compression_level );
 211

Relevant log output

No response

Please provide system details

OS Distro: Slackware
Kernel Version (uname -a): 6.2.7
System ram (free -h): lots

Additional Context

No response

@pete4abw pete4abw added the bug label Mar 21, 2023
@pete4abw pete4abw self-assigned this Mar 21, 2023
@pete4abw
Copy link
Owner Author

See this snippet from clevels.h where the strategy mapping from compression levels are defined. The headers codes WCHSLTL are defined in zstd.h

  • W-Window Log
  • C-Chain Log
  • H-Hash Log
  • S-Search Log
  • L-Match Length
  • TL-Target Length

along with numerous other parameters enumerated in ZSTD_cParameter. Potential lrzip-next compression level mappings to zstd compression levels will be based on this. More to come.

 19 #define ZSTD_MAX_CLEVEL     22
 20
 21 #ifdef __GNUC__
 22 __attribute__((__unused__))
 23 #endif
 24
 25 static const ZSTD_compressionParameters ZSTD_defaultCParameters[4][ZSTD_MAX_CLEVEL+1] = {
 26 {   /* "default" - for any srcSize > 256 KB */
 27     /* W,  C,  H,  S,  L, TL, strat */
 28     { 19, 12, 13,  1,  6,  1, ZSTD_fast    },  /* base for negative levels */
 29     { 19, 13, 14,  1,  7,  0, ZSTD_fast    },  /* level  1 */
 30     { 20, 15, 16,  1,  6,  0, ZSTD_fast    },  /* level  2 */
 31     { 21, 16, 17,  1,  5,  0, ZSTD_dfast   },  /* level  3 */
 32     { 21, 18, 18,  1,  5,  0, ZSTD_dfast   },  /* level  4 */
 33     { 21, 18, 19,  3,  5,  2, ZSTD_greedy  },  /* level  5 */
 34     { 21, 18, 19,  3,  5,  4, ZSTD_lazy    },  /* level  6 */
 35     { 21, 19, 20,  4,  5,  8, ZSTD_lazy    },  /* level  7 */
 36     { 21, 19, 20,  4,  5, 16, ZSTD_lazy2   },  /* level  8 */
 37     { 22, 20, 21,  4,  5, 16, ZSTD_lazy2   },  /* level  9 */
 38     { 22, 21, 22,  5,  5, 16, ZSTD_lazy2   },  /* level 10 */
 39     { 22, 21, 22,  6,  5, 16, ZSTD_lazy2   },  /* level 11 */
 40     { 22, 22, 23,  6,  5, 32, ZSTD_lazy2   },  /* level 12 */
 41     { 22, 22, 22,  4,  5, 32, ZSTD_btlazy2 },  /* level 13 */
 42     { 22, 22, 23,  5,  5, 32, ZSTD_btlazy2 },  /* level 14 */
 43     { 22, 23, 23,  6,  5, 32, ZSTD_btlazy2 },  /* level 15 */
 44     { 22, 22, 22,  5,  5, 48, ZSTD_btopt   },  /* level 16 */
 45     { 23, 23, 22,  5,  4, 64, ZSTD_btopt   },  /* level 17 */
 46     { 23, 23, 22,  6,  3, 64, ZSTD_btultra },  /* level 18 */
 47     { 23, 24, 22,  7,  3,256, ZSTD_btultra2},  /* level 19 */
 48     { 25, 25, 23,  7,  3,256, ZSTD_btultra2},  /* level 20 */
 49     { 26, 26, 24,  7,  3,512, ZSTD_btultra2},  /* level 21 */
 50     { 27, 27, 25,  9,  3,999, ZSTD_btultra2},  /* level 22 */
 51 },
},

@pete4abw
Copy link
Owner Author

Proposed fix in whats-next. Mapping is as follows.

W C H S L TL strat zstd level lrzip-next level
19 12 13 1 6 1 ZSTD_fast base for negative levels
19 13 14 1 7 0 ZSTD_fast level 1
20 15 16 1 6 0 ZSTD_fast level 2 1
21 16 17 1 5 0 ZSTD_dfast level 3
21 18 18 1 5 0 ZSTD_dfast level 4 2
21 18 19 3 5 2 ZSTD_greedy level 5 3
21 18 19 3 5 4 ZSTD_lazy level 6
21 19 20 4 5 8 ZSTD_lazy level 7 4
21 19 20 4 5 16 ZSTD_lazy2 level 8
22 20 21 4 5 16 ZSTD_lazy2 level 9
22 21 22 5 5 16 ZSTD_lazy2 level 10
22 21 22 6 5 16 ZSTD_lazy2 level 11
22 22 23 6 5 32 ZSTD_lazy2 level 12 5
22 22 22 4 5 32 ZSTD_btlazy2 level 13
22 22 23 5 5 32 ZSTD_btlazy2 level 14
22 23 23 6 5 32 ZSTD_btlazy2 level 15 6
22 22 22 5 5 48 ZSTD_btopt level 16
23 23 22 5 4 64 ZSTD_btopt level 17 7
23 23 22 6 3 64 ZSTD_btultra level 18 8
23 24 22 7 3 256 ZSTD_btultra2 level 19
25 25 23 7 3 256 ZSTD_btultra2 level 20
26 26 24 7 3 512 ZSTD_btultra2 level 21
27 27 25 9 3 999 ZSTD_btultra2 level 22 9

@pete4abw
Copy link
Owner Author

Fixed in main and whats-next branches.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant