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

Improve better performance #702

Closed
wants to merge 3 commits into from
Closed

Conversation

xqdoo00o
Copy link
Contributor

@xqdoo00o xqdoo00o commented Jul 20, 2020

Use larger chunkSize(64K) increase generateasync perf。
And use a robust setimmediate polyfill could increase generateasync perf of the zip with lots of chunks (min.js just 1KB larger ).
(Current setImmediate polyfill (after v3.15) using setTimeout(fn, 0) cause 4ms+ delay(browser) or 1ms delay(node.js) each chunk (current 16K size), see perf demo).

Test result for a zip with 16 files(chunks):
before:
store: 200ms
after:
store: 30ms

This PR may resolve the issue #617.

@xqdoo00o xqdoo00o changed the title Use big chunkSize improve performance Jul 20, 2020
@xqdoo00o xqdoo00o changed the title improve performance Improve performance Jul 20, 2020
@xqdoo00o xqdoo00o changed the title Improve performance Improve better performance Jul 21, 2020
@101arrowz
Copy link

@Stuk updates on this? There's a massive difference in performance between 3.1.5 and 3.5.0.

@fpaupier
Copy link

fpaupier commented Oct 5, 2021

Still having this issue with the proposed workaround. Any tips?

@101arrowz
Copy link

I'd recommend switching to a different library, at least until this is merged. zip.js and fflate (my own library) are solid alternatives.

@Stuk
Copy link
Owner

Stuk commented Oct 11, 2021

Could you merge master into this branch so that the unit tests will run? Thanks!

@xqdoo00o
Copy link
Contributor Author

Could you merge master into this branch so that the unit tests will run? Thanks!

Thanks, I merged.

@mark-westerhof
Copy link

Can this merge now?

@iiLearner
Copy link

is this being worked on anymore?

@AlttiRi
Copy link

AlttiRi commented May 18, 2022

What do you waiting for? @Stuk

set-immediate-shim is unacceptably slow.
The existence of this lib is a joke, it only provides setTimeout with "setImmediate" name. The absolutely useless "library".

Or setimmediate (of this pull request) is too big for you?


Since delay uses only the callback param of setImmediate

jszip/lib/utils.js

Lines 404 to 408 in 38f8373

exports.delay = function(callback, args, self) {
setImmediate(function () {
callback.apply(self || null, args || []);
});
};

...just use the follow code instead of importing a library:

const setImmediate = globalThis.setImmediate || /*#__PURE__*/ (function() {
    const {port1, port2} = new MessageChannel();
    const queue = [];
    port1.onmessage = function() {
        const callback = queue.shift();
        callback();
    };
    return function setImmediateLike(callback) { // Simplified implementation: only callback argument.
        port2.postMessage(null);
        queue.push(callback);
    };
})();

The implementation of setImmediate based on MessageChannel is supported everywhere:
https://developer.mozilla.org/en-US/docs/Web/API/MessageChannel#browser_compatibility
https://caniuse.com/mdn-api_messagechannel_messagechannel


ES5 version to pass linter:

var setImmediate = typeof setImmediate === "function" ? setImmediate : /*#__PURE__*/ (function() {
    var channel = new MessageChannel();
    var port1 = channel.port1;
    var port2 = channel.port2;
    var queue = [];
    port1.onmessage = function() {
        var callback = queue.shift();
        callback();
    };
    return function setImmediateLike(callback) { // Simplified implementation: only callback argument.
        port2.postMessage(null);
        queue.push(callback);
    };
})();

Just put it before 404 line (also remove the import and the dependency):

exports.delay = function(callback, args, self) {

All tests are passed. Works properly. And fast (as expected).

# pass 186
...
chromium { passed: 3301, failed: 0, total: 3301, runtime: 8940, tests: [] }
firefox { passed: 3301, failed: 0, total: 3301, runtime: 8585, tests: [] }
webkit { passed: 3301, failed: 0, total: 3301, runtime: 8367, tests: [] }

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.

None yet

7 participants