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

fix: high throughput streams #8

Merged
merged 2 commits into from
Feb 20, 2019
Merged

fix: high throughput streams #8

merged 2 commits into from
Feb 20, 2019

Conversation

jacobheun
Copy link
Contributor

@jacobheun jacobheun commented Feb 20, 2019

While testing ipfs/interop I encountered an issue with high throughput streams (file transfers). This adds a benchmark to test a large file transfer. Currently it tests a 128MB file. The performance issue was due to an improper checking of the logger being enabled. toString was being called unnecessarily, which caused a large block in IO. Details of the performance differences are below.

New Benchmark: Large file

# Before the fix
$ node large-file.js --lib=pull-mplex --repeat=1 --runs=3
sendFile*1: 6589.224ms
sendFile*1: 6592.975ms
sendFile*1: 6575.001ms

# After the fix
$ node large-file.js --lib=pull-mplex --repeat=1 --runs=3
sendFile*1: 365.876ms
sendFile*1: 299.767ms
sendFile*1: 321.524ms

pull-mplex vs libp2p-mplex

# pull-mplex
$ node large-file.js --lib=pull-mplex --repeat=1 --runs=3
sendFile*1: 403.556ms
sendFile*1: 346.207ms
sendFile*1: 306.421ms

# libp2p-mplex
$ node large-file.js --lib=libp2p-mplex --repeat=1 --runs=3
sendFile*1: 519.056ms
sendFile*1: 481.365ms
sendFile*1: 465.422ms

@ghost ghost assigned jacobheun Feb 20, 2019
@ghost ghost added the status/in-progress In progress label Feb 20, 2019
Copy link
Member

@vasco-santos vasco-santos left a comment

Choose a reason for hiding this comment

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

Wow, what an edge case!

Awesome improvements 🚀

@jacobheun jacobheun merged commit 45597f9 into master Feb 20, 2019
@ghost ghost removed the status/in-progress In progress label Feb 20, 2019
@jacobheun jacobheun deleted the fix/memory branch February 20, 2019 18:42
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.

2 participants