1

I'm been tasked w/ updating another Developer's code that was running up against governor limits (specifically the Apex CPU limit).

The code in question is fired via the before update trigger on the default contact object and is used to assign an account (if none exist) based on the contact's company name (set in a custom field).

The first part loops through the contacts and adds them to a list if they don't have an existing account Id while also building the account names set which is later used in an SOQL query:


list<Contact> newContacts               = (list<Contact>)Trigger.new;
Set<String> mailingCountries            = new Set<String>();
list<User> currentUser                  = new list<User>();
Set<String> accountNames                = new Set<String>();
list<String> excludedAccountRecordTypes = label.Excluded_Account_record_Types.split(','); // Used to limit the query

list<Contact> contactsTobeMapped = new list<Contact>();
for(Contact newCon:newContacts) {

    if(newCon.AccountId==null) {

        contactsTobeMapped.add( newCon );

        if( newCon.Temp_Company_Name__c!=null )
            accountNames.add( '%'+newCon.Temp_Company_Name__c+'%' );
        if( newCon.mailingCountryCode!=null )
            mailingCountries.add( newCon.mailingCountryCode );
    }   
}

The next part executes the SOQL query (assuming there are company names in the account names set):

if( !accountNames.isEmpty() ) {   

    list<Account> accounts = new list<Account>();

    String accountsQuery = ' SELECT Id, Name, recordtype.name, createdBy.Username, BillingCountry, BillingCountryCode FROM Account '
                            + ' WHERE RecordType.Name NOT in:excludedAccountRecordTypes '
                            + ' AND Name like :accountNames ';
    if(!mailingCountries.isEmpty()) {
        accountsQuery += ' AND BillingCountryCode!=null AND BillingCountryCode in:mailingCountries ';
    }
    accountsQuery +=' LIMIT '+50; // In order to prevent govorner limit errors

    accounts = Database.query( accountsQuery );
    ///...

The final part loops through the returned accounts and inside of that loop each of the contacts and attempts to find a match (partial or exact) between the company name (on the contact) and the account name (and updates the contact account Id if necessary):

for( Account acct: accounts ) {

    for( Contact newCon: contactsTobeMapped ) { 

            if (newCon.AccountId != null) {continue;} // Prevent reassigning accounts to contacts after they've already been assigned

            boolean accountNameExactlyMatched = newCon.Temp_Company_Name__c!=null && newCon.Temp_Company_Name__c == acct.Name;
            boolean countryNullOrMatched = ( ( newCon.mailingCountry!=null && acct.BillingCountry!=null  
                                                && newCon.mailingCountry == acct.BillingCountry ) ||
                                                ( newCon.mailingCountry == null || acct.BillingCountry == null  ) );
            boolean accountNamePartiallyMatched = ( newCon.Temp_Company_Name__c!=null && 
                                                    ( newCon.Temp_Company_Name__c.containsIgnoreCase( acct.Name ) ||
                                                        acct.Name.containsIgnoreCase( newCon.Temp_Company_Name__c ) ) );  

            if( countryNullOrMatched && ( accountNameExactlyMatched || accountNamePartiallyMatched ) ) {
                newCon.AccountId = acct.Id;
            }
        }
    }   
}

This all works as expected and brings me to my question. How can I optimize this?

I'm aware that nested for loops aren't great, but short of rewriting how he's performing the match operation, do I have other options?

As written the operation can be quite expensive from CPU perspective. If the triggering group of contacts is large and the returned accounts from the SOQL is fairly big, the amount of time required to operate can quickly escalate.

I thought about moving it over to an async (e.g. batch) execution, but my understanding is that it's better to perform field updates w/i the before trigger (so that you don't need additional DML operations).

Some of things I have done are:

1) applying a limit on the SOQL query for account (in the dev environment this is actually a dynamic variable set via a custom label) 2) added a check at the top of the inner contact loop to see if the contact had been assigned an account Id in a previous iteration (i.e. prevent reassignment)

Any ideas? I feel like there has to be some better way that I'm missing.

Thanks!

  • 1
    A few thoughts: LIKE comparisons are computationally expensive in SOQL queries, that's where a lot of time is being expended. I would set the Account LIMIT to 200 to match the trigger batch size (200 records at a time) because, worst case scenario, you have 200 Contacts that need an AccountId assigned. The AccountId check in the second Contact loop is entirely unnecessary as Contact records in contactsTobeMapped never have an AccountId and you would never run the same record through the loop again after the first time. Never use RecordType.Name, always use RecordType.DeveloperName. – nbrown Jun 3 at 16:17
  • 1
    The reason being, RecordType.Name is not necessarily unique, its translated for international users, and it can be changed at any time between environments. DeveloperName is analogous to an API name. I imagine you could see a performance boost from using DeveloperName too. – nbrown Jun 3 at 16:19
  • Thanks @nbrown! That makes sense re: matching the Account limit to trigger batch size and using RecordType.DeveloperName in place of RecordType.Name. One question - doesn't the AccountId check in the third code block prevent unnecessary AccountId reassignment. That is a contact will have a company name that matches against several accounts and so be updated multiple times. With the check, once newCon was assigned an AccountId, wouldn't it skip over it in the next loop? (seemed to in my tests but maybe I'm missing something). – Nathan House Jun 3 at 16:42
  • I see where where I messed up, the Contacts in the second code block will be iterated through for every Account. That makes me wonder if it's possible to gain efficiencies by removing the Contacts from contactsTobeMapped in some way so the set to iterate through gets progressively smaller. – nbrown Jun 3 at 17:47
  • No worries. Thanks again for the help. I had the same thought. Since I couldn't update the list as it was being iterated through, I tried a couple of variations on cloning it but the performance seemed worse so I settled on a simple break (or rather continue). – Nathan House Jun 3 at 20:25
3

You have at least two performance problems.

  1. Your query is expensive and could potentially time out. That's down to business requirements - do you have to perform a substring match for names? Does that yield you more matches than a straight equality comparison? Can you do one-time cleansing on Account names and on this field on Contact to make the matching cleaner?

  2. Your matching algorithm is an N-squared loop, but here is not optimizable using the typical Map approaches because you're doing substring matching.

Because this is an expensive operation across the board, the options that I would consider would be

  1. Reducing the complexity of the matching to make it more straightforward to optimize.
  2. Run matching in Queueable Apex, using a single Queueable invocation for each Contact and chaining until all available Contacts are processed. (Be careful to only insert one Queueable job per transaction in a trigger - if the trigger's invoked by a batch process you can hit limits otherwise).
  3. Run matching in a nightly Batch Apex job with a low(er) batch size.
| improve this answer | |
  • Thank you @david-reed. I feared this was the solution : ) I'm going to mark your answer as correct as I believe they are all good suggestions. As a quick follow-up, I assume when you say "reducing the complexity of matching", you're speaking to substring matching on names (i.e. switching it over to a straight equality comparison) ? Thanks! – Nathan House Jun 3 at 16:49
  • Yep, exactly. Or any other strategy that doesn't require substring matching. – David Reed Jun 3 at 17:06

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.