flush_interval for ReqSocket and RepSocket

[mempirate]

Same as with the PubSocket, any high-throughput socket writer should have an optional flush_interval in the options that should be respected when flushing the Framed connection. Calling flush too often adds a lot of syscall overhead as discussed in #11.

The default here should actually be None imo, and flushing should happen after every send, since the usual behaviour for request response (at least in our case) will be low-latency and low throughput. But it should be configurable for other use cases nonetheless. The benchmarks should also play around with this option.

Tasks

Beta Give feedback

1. Add flush_interval field in ReqOptions, default None
2. Add flush_interval field in RepOptions, default None
3. Respect flush_interval in rep::PeerState
4. Respect flush_interval in req::Driver

[PatStiles]

Interested!

[mempirate]

Assigned! You can take a look at how it's done in the publisher here: #24

[quangkeu95]

Is this issue still available? I can work on this

[mempirate]

@PatStiles if you currently don't have the bw for this, let me know and I'll assign @quangkeu95

[quangkeu95]

I tried to create a PR here, so any feedbacks is appreciated!

[quangkeu95]

what is your device spec that you are using to run the benchmark @jonasbostoen ? I tried to run the benchmark for reqrep but cannot achieve the results in the msg/benches/reqrep.rs file. Currently running on Macbook Pro 2019 with Intel i9 2.3Ghz 8-Core

[mempirate]

@quangkeu95 running on M1 Macbook Pro.

I ran the benchmark again, and when setting flush_interval to 50 microseconds on the ReqSocket, I get a pretty good throughput improvement:

Multi-threaded benchmarks
Before: 249.6 ms, 102.5 MB/s, 200.2 Kitem/s
After: 197.8ms, 129.4 MB/s, 252.7 Kitem/s

However, when I set the option on the RepSocket, throughput tanks by around 70%. I'm not sure why, and will investigate later, but for now I think we should remove the option on RepOptions.

[quangkeu95]

Let me take a look, if the performance is decreased then we could consider remove the option in RepOptions.

[mempirate]

Fixed by #30