Skip to content
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

CRM-21108 - Greatly improve performance of adding contacts #10906

Merged
merged 3 commits into from
Aug 29, 2017

Conversation

MegaphoneJon
Copy link
Contributor

@MegaphoneJon MegaphoneJon commented Aug 27, 2017

Overview

It currently takes 21 seconds to add 200 contacts using CRM_Contact_BAO::create(). #10905 shaves off 4 seconds from that; this PR shaves off another 10, resulting in a 2x to 3x speedup in creating contacts.

Before

Creating 200 contacts takes ~17 seconds (after #10905 is applied), greeting processing takes 13.6 seconds:
image

After

Creating 200 contacts takes 6.5 seconds, greeting processing takes 3.0 seconds:
selection_227

Technical Details

Greetings tokens are calculated by loading a contact's data into an array (via DAO->find()), replacing tokens with that data, then checking if there are any unreplaced tokens. If any tokens are unreplaced, Civi makes two (very expensive!) calls to CRM_Contact_BAO_Query. Since non-contact tokens in greetings are rare, this should happen only rarely.

Unfortunately, the greetings code doesn't account for the fact that fields that are NULL in the database won't appear in the object created by DAO->find(). Since the default addressee greeting has 5 tokens, you end up with a very slow greeting replacement unless your contacts have all of the following: prefix, suffix, first name, middle name, last name.

Finally, there need not be two calls to CRM_Contact_BAO_Query; one is sufficient.

By identifying and removing contact tokens whose values are null, we avoid the expensive calls. On the rare occasion such a call is needed, we still save ~6 seconds by calling the function once instead of twice.

Comments

You don't need a profiler to confirm this; you can download this file, rename to contactprofiler.php, save it in your civiroot, and run it with php contactprofiler.php.
contactprofiler.php.txt


@MegaphoneJon
Copy link
Contributor Author

Test failure is related. I ran CRM_Utils_Token and CRM_Contact_BAO tests, but not the whole test suite because of the overhead (which hopefully this PR will reduce!). I'll take a look and deal with this.

@totten
Copy link
Member

totten commented Aug 28, 2017

I haven't dug into the patch, but a general comment -- that looks like a really good investigation into why bulk-addition becomes slow.

@monishdeb
Copy link
Member

Here's the time difference on running provided php script contactprofiler.php, before and after applying the patch:

  1. BEFORE
    Start Time: 22:37:28
    End Time: 22:37:38
    Difference - 10sec

  2. AFTER
    Start Time: 22:38:10
    End Time: 22:38:14
    Difference - 4sec

Also, checked the underlying code and tested the patch on my local. Working fine, didn't cause any unintended regression.

@monishdeb monishdeb merged commit 0857857 into civicrm:master Aug 29, 2017
@MegaphoneJon MegaphoneJon deleted the CRM-21108-2 branch October 10, 2017 02:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants