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

Include zstd compression & decompression in serde #203

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

CompeyDev
Copy link
Contributor

Closes #202.

Tests are broken since the generated ZStd file by the CLI does not match the one generated by serde. Not really sure why.
Copy link
Collaborator

@filiptibell filiptibell left a comment

Choose a reason for hiding this comment

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

Looks good so far! Just the tests failing, does the zstd cli need some args changed maybe?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Was the formatting here changed on accident? This file should definitely be less than 80 wide (prettiers default) and not have to wrap across lines

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, that must've been what happened - I ran prettier on root and it didn't respect the config perhaps?

@CompeyDev
Copy link
Contributor Author

CompeyDev commented Jun 3, 2024

Looks good so far! Just the tests failing, does the zstd cli need some args changed maybe?

I've tried most permutations of the arguments that I can pass to the CLI - async-compression's best mode corresponds on ZSTD's MAX_CLEVEL, which is 22 (I've read the source of both the crate and zstd to confirm this).

@filiptibell
Copy link
Collaborator

Did you figure out what could be going on here? Not really sure myself but I also haven't looked too deeply into it.

@CompeyDev
Copy link
Contributor Author

Did you figure out what could be going on here?

Unfortunately no :( Others in the lune channel could not figure it out either.

@CompeyDev CompeyDev changed the title feat(serde): zstd compression & decompression Include zstd compression & decompression in serde Oct 17, 2024
@Dekkonot
Copy link
Contributor

According to the zstd CLI tool (downloaded from the zstd repo) the best compression level is 19. Are we sure that 22 is the max it uses?

@qwreey
Copy link
Contributor

qwreey commented Oct 28, 2024

According to the zstd CLI tool (downloaded from the zstd repo) the best compression level is 19. Are we sure that 22 is the max it uses?

between 1 and 19; << In normal mode
They provide '--ultra' extra option (Enable levels beyond 19, up to 22; requires more memory.)
Steps 20 and above work a little differently and are slower

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

Successfully merging this pull request may close these issues.

Add support for zstd Compression Algorithm to @lune/serde
4 participants