asmcrypto / asmcrypto.js

JavaScript Cryptographic Library with performance in mind.
MIT License
660 stars 182 forks source link

grunt (really uglify) fails on sha512 #75

Closed ktarplee closed 9 years ago

ktarplee commented 9 years ago

grunt --with=sha512

Running "sources" task Building modules: sha512

Running "uglify:release" (uglify) task [RangeError: Maximum call stack size exceeded]

Uglifying source src/errors.js,src/utils.js,src/hash/hash.js,src/hash/sha512/sha512.asm.js,src/hash/sha512/sha512.js,src/hash/sha512/exports.js failed. Warning: Uglification failed. Maximum call stack size exceeded. Use --force to continue.

Aborted due to warnings.

I tried increasing the stack trace but no luck. It keeps eating up memory an not completing.

node --stack-size=10000000 ./node_modules/.bin/grunt sha512 Warning: Task "sha512" not found. Use --force to continue.

Aborted due to warnings. kmt-work:asmcrypto.js kmtarplee$ node --stack-size=10000000 ./node_modules/.bin/grunt --with=sha512 Running "sources" task Building modules: sha512

Running "uglify:release" (uglify) task ^C

ktarplee commented 9 years ago

I noticed you have your own uglify. I know infinite recursion was an issue in the upstream uglify so maybe that needs to be ported to yours?

vibornoff commented 9 years ago

Unfortunately I can't reproduce this bug with my node v0.10.25. Nonetheless I see my travis build failing with node v0.11.16 exactly due to this problem.

Just updated my uglify version with the upstram. Try it now.

Note, maybe you'll have to manually reinstall local uglify-js package with rm -rf node_modules/uglify-js && npm install

ktarplee commented 9 years ago

I am using node v0.12.2 (via homebrew on the Mac). Which branch do you want me to test? release or master

If I am on master and I change the package.json file: "uglify-js": "=2.4.20",

rm -rf node_modules npm install

grunt --with sha512

Running "sources" task Building modules: sha512

Running "uglify:release" (uglify) task [RangeError: Maximum call stack size exceeded]

Uglifying source src/errors.js,src/utils.js,src/hash/hash.js,src/hash/sha512/sha512.asm.js,src/hash/sha512/sha512.js,src/hash/sha512/exports.js failed. Warning: Uglification failed. Maximum call stack size exceeded. Use --force to continue.

Aborted due to warnings.

ktarplee commented 9 years ago

I updated uglify-js and grunt-contrib-uglify to the latest (also bumped grunt and grunt-cli but I don't think those matter). That seems to work. "grunt --with sha512" now works.

Compression in uglify takes a long time (minutes for the above command). Disabling compression speeds it up grunt considerably. Is compression really necessary? The file is only 74KB with compression and 75KB without compression in the sha512 only case.

ktarplee commented 9 years ago

Disabling compression with the old uglify works as well. You can leave compression enabled if you update the uglify and the grunt-contrib-uglify modules.

vibornoff commented 9 years ago

About compression, @mishoo didn't approved the pull request #373 yet, so vanilla uglify-js breaks asm.js code (though it's still valid js code it doesn't pass asm.js verification and its performance dramatically degrades at least in Firefox). And that's why the compression takes so a long time.

You're right, when compression is turned off, vanilla uglify-js works fine. But it worth being turned on. With the latest patched uglify-js compressed asmcrypto.js results in 128KB vs 296KB uncompressed one.

That's why I have to maintain my own uglify-js untill @mishoo lets the patch in. You shouldn't change devDependencies in the package.json, uglify-js dependancy must be "vibornoff/UglifyJS2".

vibornoff commented 9 years ago

Damn, it still fails. https://travis-ci.org/vibornoff/asmcrypto.js/builds/57720786

vibornoff commented 9 years ago

Trying another way with https://github.com/vibornoff/asmcrypto.js/commit/bab07e11d1ab9e558a4768ef48258caf3599f695

vibornoff commented 9 years ago

Seems it work now https://travis-ci.org/vibornoff/asmcrypto.js/builds/60597335

ktarplee commented 9 years ago

I'm not sure why you closed this as your code does not work on the latest node (0.12.2). "grunt --with sha512" is still failing unless you set mangle and compress to false in the Gruntfile