Skip to content

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.

3 participants