veda-consulting-company / uk.co.vedaconsulting.mailchimp

Other
22 stars 43 forks source link

Review use of sleep() #243

Open xurizaemon opened 7 years ago

xurizaemon commented 7 years ago

There are a couple of places currently where I see the code uses sleep() and I wanted to open a discussion about how they could be made more efficient. This is probably only of interest to sites busy enough to care about efficiency - sleep() is cheap() 😈

CRM_Mailchimp_Page_WebHook::profile()

AFAICT this is because we expect to get hit twice for a new contact signup over on Mailchimp's forms, once for ::subscribe() and once for ::profile(). I haven't yet dug into how Mailchimp deliver these yet, or whether it's still necessary to process both separately, or what the timing is, or whether ::subscribe() could just query MC for the profile data ...

On seeing this it felt like there's potential for optimising, by replacing sleep(10) with a loop that sleeps and attempts to query for the to-be-created contact. If it takes 2s to create the contact, we can discover that and complete our work at 3s instead of sleeping longer ...

CRM_Mailchimp_Api3::batchAndWait()

This looks like it's there to avoid hitting a rate limit of some kind? Mailchimp's v2 docs suggest that there is none currently unless you are a bad monkey, and their v3 docs don't say anything on it. Is it to avoid rate limiting, or for something else?

I'm flagging these as "interesting", just 'cos they felt like they didn't "gel" with other areas where performance had been given quite a bit of attention. I don't think they require urgent attention, but I'm keen to discuss if there's anyone to discuss it with, and now I have a place to make notes if I get a chance to observe some subscribe-and-profile hits ...