Skip to content
This repository has been archived by the owner on Feb 24, 2021. It is now read-only.

Heavy browser performance impact #92

Closed
Beanow opened this issue Sep 3, 2017 · 11 comments
Closed

Heavy browser performance impact #92

Beanow opened this issue Sep 3, 2017 · 11 comments

Comments

@Beanow
Copy link

Beanow commented Sep 3, 2017

Performing a test by ipfs.files.get on a larger file, such as a 4MB audio file. Secio processes roughtly 7,5MB of data with AES CTR-mode decrypt here https://github.com/libp2p/js-libp2p-secio/blob/master/src/etm.js#L59.

This process took 249 seconds on a fresh repo (downloading from another tab), 243 seconds of which was spent in Firefox's garbage collection (v55, 64bit Linux) cleaning up the buffers created for decryption. All the while blocking the main thread.

Whereas Chromium consistently farts out.
image

This is a profile where the file was offered by https://orbit.chat/ and downloaded by a different tab.
It doesn't cover the entire 249 seconds, as samples were already being discarded before then.
profile-secio.json.zip

@daviddias
Copy link
Member

Thank you for the throughout report, @Beanow 5🌟

There is a fix incoming specifically for this -- browserify/browserify-aes#48 -- waiting for a release!

@ya7ya
Copy link

ya7ya commented Sep 3, 2017

Hey @Beanow , Thanks for reporting, this is an issue with browserify-aes browserify/browserify-aes#48 , and it got fixed but we're still waiting for release, since this is the main culprit of the issue.

@ya7ya
Copy link

ya7ya commented Sep 3, 2017

@Beanow Till it gets released, I made a fork that uses the unreleased version of browserify-aes but this is not suitable for anything beyond testing. so please don't use it in production.

@Beanow
Copy link
Author

Beanow commented Sep 3, 2017

👍 sweet, using the fixed version cuts the same process down to 32 seconds. It went down from >90K GC calls to 3400.

Edit: cleaning things up more I got down to 2-4 seconds with the fix.

@Beanow
Copy link
Author

Beanow commented Sep 3, 2017

Thanks @ya7ya, what I did though was put a dependency to "browserify-aes": "github:crypto-browserify/browserify-aes#master" in my project root and webpack is happy to use it.

@ya7ya
Copy link

ya7ya commented Sep 3, 2017

Great, all credit goes to @dignifiedquire :D

@haadcode
Copy link

haadcode commented Sep 4, 2017

Woah! If this fixes what I think it'll fix, this will be a major improvement to the performance. Those ^ numbers sound amazing :) Great work everyone involved, thank you!

@ya7ya
Copy link

ya7ya commented Sep 5, 2017

@diasdavid since libp2p-crypto is now released and uses browserify/browserify-aes#48 , Can We get libp2p-secio release to use the new libp2p-crypto version?

@daviddias
Copy link
Member

Not yet, there was an issue with the update to browserify-aes. it got mixed with the no unsafe-eval endeavour) ipfs/js-ipfs#991 which resulted in this big problem ipld/js-ipld-dag-pb#41). I had to unpublish it.

Let me revert the switch to protobufjs done here https://github.com/libp2p/js-libp2p-crypto/commits/master and release it.

@daviddias
Copy link
Member

@ya7ya libp2p-crypto@0.10.2 was published with the new browserify-aes. A fresh npm install (don't forget to delete package-lock.json) should get you a less memory hungry IPFS :)

@Beanow
Copy link
Author

Beanow commented Sep 9, 2017

Tested a fresh npm install. This resolved to browserify-aes 1.0.8 and fixes the issue.
Thanks all! It's not every day you get 💯x performance boosts days after reporting.

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

No branches or pull requests

4 participants