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

unpack: open file handle only once #142

Closed
wants to merge 1 commit into from
Closed

Conversation

zkat
Copy link
Contributor

@zkat zkat commented Sep 17, 2017

This gives a significant speed boost, specially when doing chown and utimes operations, by reducing the number of fd open+close ops.

Note: this PR is still WIP while I figure out why tests are failing. It looks like error handling needs to change and the stuff you see for handling errors right now is mostly me flopping around trying to figure out the issue.

This PR is inspired by yarnpkg/yarn#4486, which did this for Yarn's own extraction and it had a pretty neat speed boost! 🎉

Benchmarks

Before

benchmarks/extract/node-tar-file-async.js
791.322
844.052
887.71
898.871
924.761
benchmarks/extract/node-tar-stream-sync.js
1089.522
1122.212
1077.03
1045.234
1009.606

After

benchmarks/extract/node-tar-file-async.js
619.741
654.146
638.94
657.804
708.489
benchmarks/extract/node-tar-stream-async.js
793.267
779.895
736.296
718.423
777.464

@isaacs
Copy link
Owner

isaacs commented Dec 20, 2017

Conflicts with fsm update, closing.

Copy link

@nowakwp nowakwp left a comment

Choose a reason for hiding this comment

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

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.

3 participants