vert-x/vert.x — pull request #471 by npahucki (2012-12-20 21:26:30+00:00)
Title: Fixes for HttpClient connection pooling 

HttpClient connection pooling seems to be broken in 1.2.3.final, and in many cases turning keep-alive off improves performance significantly. This is especially noticeable when some requests are longer in nature. The root of the problem seems to be that as soon as the request is written on a connection, it goes back into the pool, where another request can be assigned to it. Of course, each request/response pair has to complete serially, which means that if every request assigned to a connection has to wait for all the requests before it to complete. Since the connection pool pops the head of the available connection collection, in some cases, the same connection is used over and over, having many requests assigned to it, when really it would be better to be a little more intelligent, and pick an idle connection (as in it has no pending requests) from the pool, or failing any idle connections, the one that has the fewest outstanding pending requests. This would still allow one long request to block others on the same connection, but it spreads the probability over all the connections in the pool instead of on a few at the head of the queue.

Below are some tests I did before and after the proposed changes (source for tests attached). You'll notice that when latency is low, with the old code, keep-alive on performs only about half as well as not using keep-alive. As the latency increases, keep-alive starts showing benefit. With my changes, you can see that keep-alive performs the same as *no* keep-alive in low latency, and much better in higher latency situations. All these tests are with SSL off, but I did do some off the cuff tests with SSL, and keep-alive has a very big benefit in that case as the handshake doesn't need to be done again.

I did one experiment, which is still present in the code for your inspection (which I expect that you'll take it out during the pull/merge) was designed to put threads in the waiter collection if there are no unoccupied connections (connections with outstanding requests). Running in this mode completely kills performance though, but I can't understand why. It would seem that every time a request completed, the connection would get returned to the pool and the next waiter run. My reasoning for testing this mode was, in the default mode, if a request gets assigned to a connection with a longish running request it is delayed (for the time the request before it takes) before it can even begin, whereas if a request waited for the first available connection, things would be better since shorter requests would return their connection to the pool very fast, letting the a waiting request proceed in the time is takes the shortest currently outstanding request to complete and not some arbitrary time of the request in front of it. What do you think of this? What am I missing here?

Finally, one interesting thing I observed and would like guidance on, is that the number of boss threads seems to have no effect what-so-ever on the performance tests I was doing. Setting it to 1 or 100 produced the exact same numbers. This seems counter intuitive, but maybe I'm not understanding what these threads do. Any ideas?

The Results:

boss_threads =5, max_pool_size = 25, requests per client = 500, request_process_ms=100;
Results Before Modifications:
Localhost ~0ms Latency
Keep Alive Off, Warm AVG:1257 TOTAL TIME: 2403 REQUEST/SEC:208
Keep Alive On, Warm AVG:2043 TOTAL TIME: 4076 REQUEST/SEC:122
Simulated Latency 2ms
Keep Alive Off, Warm AVG:2545 TOTAL TIME: 4356 REQUEST/SEC:114
Keep Alive On, Warm AVG:2074 TOTAL TIME: 4102 REQUEST/SEC:121
Simulated Latency 90ms
Keep Alive Off, Warm AVG:9496 TOTAL TIME: 18135 REQUEST/SEC:27
Keep Alive On, Warm AVG:2538 TOTAL TIME: 4666 REQUEST/SEC:107

After fixes…
Localhost ~0ms Latency
Keep Alive Off AVG:11830 TOTAL TIME: 24111 REQUEST/SEC:208
Keep Alive On AVG:11810 TOTAL TIME: 24111 REQUEST/SEC:208
Simulated Latency 2ms
Keep Alive Off, Warm AVG:2636 TOTAL TIME: 4459 REQUEST/SEC:112
Keep Alive On, Warm AVG:2008 TOTAL TIME: 3986 REQUEST/SEC:125
Simulated Latency 90ms
Keep Alive Off, Warm AVG:9486 TOTAL TIME: 18105 REQUEST/SEC:27
Keep Alive On, Warm AVG:2442 TOTAL TIME: 4719 REQUEST/SEC:105

boss_threads =5, max_pool_size = 25, requests per client = 500, request_process_ms=200;
Results Before Modifications:
Localhost ~0ms Latency
Keep Alive Off, Warm AVG:2301 TOTAL TIME: 4397 REQUEST/SEC:113
Keep Alive On, Warm AVG:3763 TOTAL TIME: 7476 REQUEST/SEC:66
Simulated Latency 2ms
Keep Alive Off, Warm AVG:3310 TOTAL TIME: 5991 REQUEST/SEC:83
Keep Alive On, Warm AVG:3779 TOTAL TIME: 7485 REQUEST/SEC:66
Simulated Latency 90ms
Keep Alive Off, Warm AVG:12118 TOTAL TIME: 22032 REQUEST/SEC:22
Keep Alive On, Warm AVG:4079 TOTAL TIME: 7684 REQUEST/SEC:65

After fixes…
Localhost ~0ms Latency
Keep Alive Off, Warm AVG:2299 TOTAL TIME: 4394 REQUEST/SEC:113
Keep Alive On, Warm AVG:2288 TOTAL TIME: 4399 REQUEST/SEC:113
Simulated Latency 2ms
Keep Alive Off, Warm AVG:3385 TOTAL TIME: 6042 REQUEST/SEC:82
Keep Alive On, Warm AVG:3643 TOTAL TIME: 7285 REQUEST/SEC:68
Simulated Latency 90ms
Keep Alive Off, Warm AVG:10529 TOTAL TIME: 20134 REQUEST/SEC:24
Keep Alive On, Warm AVG:4074 TOTAL TIME: 7683 REQUEST/SEC:65