Skip to content

Conversation

@Stieneee
Copy link

Thanks for the package.

Wanted an update to date implementation of zstd running in node 10. Some nan api definitions had deprecated resulting in failed compilation.

With some stress testing I was getting some seg faults. Stack traces lead me into nan. After moving the output buffer and removing the sync worker->Destroy() call everything is behaving correctly. I confirmed that the worker is still being destroyed properly after a sync call.

Some of the tests were missing some done calls.

change to nan realted calls to support node 10
fixed incorrect tests
moved out buffers from worker
removed destroy call as was causing seg fault
increase mocha timeout for slow ci performance
@paambaati
Copy link

@zwb-ict Bump!

Copy link

@ethercflow ethercflow left a comment

Choose a reason for hiding this comment

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

LGTM

@dvtate
Copy link

dvtate commented Jul 7, 2021

sad

@paambaati
Copy link

If someone is looking for decompressing zstd, you might want to check out https://github.com/101arrowz/fzstd. It is built in pure-JavaScript and is pretty fast.

@Stieneee
Copy link
Author

Stieneee commented Jul 7, 2021

@paambaati looks interesting thanks for the share.

After this failed to merge I actually created a new package that uses the zstd binary.

simple-zstd

My belief is that spawning the second process is more resource-friendly than an in javascript implementation. The package should also be more resilient to Node version changes and easier to keep up-to-date with current zstd. Docs should probably be updated to use the pipeline method. I have been using simple-zstd for a while now in my applications and it has been reliable.

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.

4 participants