Performance: reuse cURL connections

ewandennis commented 7 years ago

This PR holds a cURL session open so it can reuse connections. The effect is to amortise connection setup cost across HTTP requests to the same host.

Changes:

renamed cURL session var $ch to $curl_session
moved $curl_session into a static class variable (self::$curl_session)
Conditionally create $curl_session on first use
Remove call to curl_close() to enable session reuse
Call sparkpost_log() to report on total call time for each request

nganivet commented 7 years ago

Ewan,

Thanks for your contribution. I had already made similar changes in the 'optimize' branch (https://github.com/cividesk/com.cividesk.email.sparkpost/compare/optimize), and they did not lead to much improvements as the curl() functions already cache connections by default. So this is just making the cache explicit, but it already is in effect underneath.

I will send you more data, but I do not think the performance issue is related to SSL session establishment.

--Nicolas.

ewandennis commented 7 years ago

My understanding of cURL is that it only caches connections within a session, so when using a single session variable created by curl_init(). I found two main StackOverflow discussions on the topic: here and here and an FAQ from the cURL project here.

My own test code is here with the following results. The second set has connection reuse enabled:

total,name lookup,connect,pre transfer,start transfer,size upload,size download,speed download,speed upload
0.711176,0.005427,0.176076,0.532575,0.71115,830,108,151,1167
0.509758,2.7E-5,0.174326,0.342118,0.509738,830,108,211,1628
0.550593,1.7E-5,0.203839,0.375935,0.550567,830,109,197,1507
0.652138,2.5E-5,0.171623,0.339576,0.652114,830,108,165,1272
0.535157,2.6E-5,0.18888,0.356669,0.535131,830,108,201,1550
total,name lookup,connect,pre transfer,start transfer,size upload,size download,speed download,speed upload
0.516302,2.6E-5,0.173058,0.345606,0.516278,830,109,211,1607
0.170434,3.0E-5,3.1E-5,0.000108,0.170409,830,108,633,4869
0.168908,3.1E-5,3.2E-5,0.000108,0.168889,830,109,645,4913
0.17174,2.1E-5,2.1E-5,7.6E-5,0.171715,830,108,628,4832
0.173741,3.4E-5,3.4E-5,0.000104,0.173715,830,109,627,4777

nganivet commented 7 years ago

Ewan - I get your point, but even with connection reuse you still have 0.17 seconds to send an email ... that is max 6 emails/sec ... so API performance is definitely poor!

On a side note, the difference in the SSL connect and send (.32 for me, .17 for you) is also fishy as the endpoint is in AWS us-west, and I do my tests from AWS us-east. You live in the UK, but where do you do your tests from?

------ Original Message ------ From: "Ewan Dennis" notifications@github.com To: "cividesk/com.cividesk.email.sparkpost" com.cividesk.email.sparkpost@noreply.github.com Cc: "nganivet" nicolas@cividesk.com; "Comment" comment@noreply.github.com Sent: 11/15/2016 12:19:02 PM Subject: Re: [cividesk/com.cividesk.email.sparkpost] Performance: reuse cURL connections (#37)

My understanding of cURL is that it only caches connections within a session, so when using a single session variable created by curl_init(). I found two main StackOverflow discussions on the topic: here and here and an FAQ from the cURL project here.

My own test code is here with the following results. The second set has connection reuse enabled:

total,name lookup,connect,pre transfer,start transfer,size upload,size download,speed download,speed upload 0.711176,0.005427,0.176076,0.532575,0.71115,830,108,151,1167 0.509758,2.7E-5,0.174326,0.342118,0.509738,830,108,211,1628 0.550593,1.7E-5,0.203839,0.375935,0.550567,830,109,197,1507 0.652138,2.5E-5,0.171623,0.339576,0.652114,830,108,165,1272 0.535157,2.6E-5,0.18888,0.356669,0.535131,830,108,201,1550 total,name lookup,connect,pre transfer,start transfer,size upload,size download,speed download,speed upload 0.516302,2.6E-5,0.173058,0.345606,0.516278,830,109,211,1607 0.170434,3.0E-5,3.1E-5,0.000108,0.170409,830,108,633,4869 0.168908,3.1E-5,3.2E-5,0.000108,0.168889,830,109,645,4913 0.17174,2.1E-5,2.1E-5,7.6E-5,0.171715,830,108,628,4832 0.173741,3.4E-5,3.4E-5,0.000104,0.173715,830,109,627,4777 — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

ewandennis commented 7 years ago

My test results above include transatlantic latency which I think is 70-80ms. Here's another anecdotal test running from an EC2 instance in us-east:

total,name lookup,connect,pre transfer,start transfer,size upload,size download,speed download,speed upload
0.320844,0.001091,0.075005,0.238761,0.320814,830,109,339,2586
0.360433,1.5E-5,0.083074,0.262634,0.360417,830,109,302,2302
0.465027,1.6E-5,0.098114,0.307584,0.465009,830,109,234,1784
0.295876,1.5E-5,0.068317,0.21761,0.29586,830,108,365,2805
0.360749,1.6E-5,0.084183,0.265872,0.360733,830,108,299,2300
total,name lookup,connect,pre transfer,start transfer,size upload,size download,speed download,speed upload
0.335197,1.6E-5,0.077394,0.245627,0.335181,830,109,325,2476
0.088417,1.9E-5,2.1E-5,5.6E-5,0.088401,830,108,1221,9387
0.088633,1.6E-5,1.8E-5,5.2E-5,0.088617,830,108,1218,9364
0.088195,1.5E-5,1.7E-5,5.3E-5,0.088179,830,109,1235,9410
0.096591,1.5E-5,1.7E-5,5.1E-5,0.096575,830,108,1118,8592

cividesk / com.cividesk.email.sparkpost

Performance: reuse cURL connections #37