How to Optimize a Nested For Loop and Avoid CPU Limit Errors

Question

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!

Answer 1

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.