-
Notifications
You must be signed in to change notification settings - Fork 48
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
perf: decrease wantlist send debounce time #224
Conversation
We wait 200ms before sending wantlists to remote peers which is an eternity. I'm not sure how this figure was arrived at but I've reduced it to 10ms which has increased the js->js transfer times in the interop tests to: | Data | 200ms | 10ms | Speedup | |---------|----------|---------|---------| | 1.02 kB | 486ms | 142ms | 342% | | 63.5 kB | 449ms | 119ms | 377% | | 65.5 kB | 436ms | 105ms | 415% | | 524 kB | 1287ms | 210ms | 613% | | 786 kB | 1676ms | 282ms | 594% | | 1.05 MB | 2066ms | 326ms | 634% | | 1.05 MB | 2123ms | 330ms | 643% | | 4.19 MB | 7091ms | 1045ms | 679% | | 8.39 MB | 13711ms | 1927ms | 712% | | 67.1 MB | 107704ms | 13462ms | 800% | | 134 MB | 214988ms | 26038ms | 826% |
Running with different debounce times and counting the network packets transmitted between nodes (pinning is disabled so the overall time is a lot less):
By comparison, go-ipfs@0.5.1 running the same test:
Based on this we should probably disable the debounce entirely. |
…ist-send-debounce
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
I removed a similar 100ms debounce from the engine (bitswap server side) recently.
The reason it may be useful to have a small debounce on the client is that when sending wants it's better to send 10 wants in a single wantlist rather than 10 messages with an individual want in each one. In go-bitswap we use a 1ms debounce.
We debounce sending wantlists to remote peers by 200ms which is an eternity. I'm not sure how this figure was arrived at but I've reduced it to 10ms which has increased the js->js transfer times in the "exchange files" interop tests to: