-
Notifications
You must be signed in to change notification settings - Fork 191
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
Switch from network to socket #306
Conversation
I wrote a little benchmark to make a bunch of requests on a single thread as fast as possible: https://gist.github.com/tfausak/afab233786cee3090f55974ffe0b1ba8/062b0a042b971c7c1f5f0341d4fc08d9e842535a The initial results are very good! So good, in fact, that I feel like I must have done something wrong.
Is it possible that I also saw some crashes ( |
@tfausak Have you used the threaded or non-threaded GHC runtime? |
Alright, I updated my benchmark to be a little more like the real world and a little less synthetic: https://gist.github.com/tfausak/afab233786cee3090f55974ffe0b1ba8/b5e3a58e342b66658ff386529bf6ad895ba92e8b The order of magnitude difference is gone. Both branches now perform basically the same.
I ran those benchmarks with these |
This change set is surprisingly small. I'll take a closer look at this later. Looks pretty nice at a glance. |
I'd love to see some profiling and threadscope displays of these benchmarks. I'm really curious if the performance gains are coming from less allocation via |
The other interesting thing to test would be a benchmark that generates high contention between writing threads. That is an area where the |
Pulling in @kazu-yamamoto for curiosity. |
I'm not familiar with thread scope, but I can do some profiled runs. Any ideas for how to create a workload that would have a lot of contention? My naive guess is to basically do the same thing as my linear benchmark except make the requests on N threads. |
I went ahead and added the benchmark to this branch. I also updated it to use more threads. Then I ran it against master and this branch — they're still really close. I also ran the benchmarks with profiling enabled. Assuming I did all this correctly (which is a big assumption), |
Veeeerrrrry interesting. We avoided adding an |
@eborden You should open an issue on the network package. |
How is the powdered wiggery going? |
Granted 3.0.0.0 is purely theory and intended as a pie in the sky project. |
I'm going to close this. I never intended for this to be merged. The excellent maintainers of |
This pull request explores switching from the venerable
network
package to @lpeterse'ssocket
package. The switch turned out to be easier than I expected. Nevertheless, a few of the tests fail:I didn't spend much time trying to debug those because they seem to rely on
streaming-commons
, which itself depends onnetwork
and exposesSocket
s.My goal here isn't actually to replace
network
withsocket
— at least not right away. I wanted to see if it was possible and how it affected performance (sincesocket
usesMVar
s behind the scenes). Sadly I don't have an off-the-shelf way to measure the performance ofhttp-client
. I'll have to look around for one unless someone has a better idea. I tried runninghttp-client/bench/threaded-stress.hs
, but after bringing it up to date I realized it doesn't really stress test the sockets.cc @eborden @snoyberg