6

One of my old triggers is beginning to cause Apex CPU Time Limit errors, I was fairly close to this limit when the trigger was first made but since it worked perfectly for so long it was neglected.

The first thing it does it build a Map:

Map<String,Id> comparisonMap = new Map<String,Id>();
for(Lead l : [Select Id, Company, Postcode__c, Landline_Phone__c FROM Lead WHERE isConverted = FALSE AND Check_Lead__c = TRUE]){

    IF(l.Company != null){
        comparisonMap.put(l.Company.toLowerCase().replace(' ','').replace('-',''), l.Id);
    }
    IF(l.Postcode__c != null){
        comparisonMap.put(l.Postcode__c.replace(' ',''), l.Id);
    }
    IF(l.Landline_Phone__c != null){
        comparisonMap.put(l.Landline_Phone__c.replace(' ',''), l.Id);
    }

}

Which is then added to a set for later use:

Set<String> comparisonSet = new Set<String>();
comparisonSet.addAll(comparisonMap.keySet());

Is there a more efficient way of doing this? I think the .toLowerCase() and .replace methods on line 5 are quite taxing on the CPU Time:

comparisonMap.put(l.Company.toLowerCase().replace(' ','').replace('-',''), l.Id);

Any help would be greatly appreciated!

1
  • why do you need the comparisonSet? map's keys are unique and do not have dupes, having said that can't you just use comaprisonMap.keySet() ?
    – Rao
    Commented Apr 7, 2016 at 18:57

2 Answers 2

12

There's no shortcut to what you're doing, but here's a few probable optimizations you can perform.

Use Three Maps

Maps are much faster than manually searching lists (at least 10 times faster on average), partly because they're internally optimized to be faster than linear search algorithms. However, they do not scale as well as lists do. Maintaining three smaller maps should yield better performance.

Use String.replaceAll

Function calls are technically more expensive than simple regexp patterns, so it makes sense to reduce the number of calls you need to make:

l.Company.toLowerCase().replaceAll('[- ]','')

Use Set.clone() When Possible

If you're still using just one map, do this:

Set<String> comparisonSet = comparisonMap.keySet().clone();

This is faster than constructing a new object and using addAll. If you go with three maps, however, you'll still need to use addAll twice to get all the strings in your master set.

References Are Expensive

Every time you need to use a . to deference something, it's costing you CPU time. I'm not sure when the balance between when to switch between direct references to caching variables (because lines are also expensive), but you might try it out:

for(Lead l...) {
    Id leadId = l.Id;
    // Use leadId for the rest of this block
}

Use Internal String Methods, When Possible

You can replace .replace(' ','') with .deleteWhitespace(); this may or may not be more efficient for the Company field, since you're still left with a need to replace one character (the - character), but it's probably better for the other two fields.

2
  • Really well explained and solid information, thanks for taking your time to write this. I've been messing around with my trigger and managed to reduce CPU time by almost 20% already by using a few of these tips. Suppose I didn't need the ID and just build a set comparisonSet.add(l.Company.deleteWhiteSpace()); Would it be more efficient to use 3 separate Sets for these or 1 master Set for all the information?
    – Liam
    Commented Apr 7, 2016 at 22:51
  • Wow, problem solved! I had a chance to quickly try using 3 Sets and performance has massively increased. Thanks again I learned a lot from your reply.
    – Liam
    Commented Apr 7, 2016 at 23:24
0

I know this has been answered already and this may not help. In your code you are only adding Leads to your map where (Company != null or Postcode__c != null or Landline_Phone__c != null).

I don't know what the rest of your code looks like or if it would help but would it be worth making your initial query more selective?

eg:

for(Lead l : [Select Id, Company, Postcode__c, Landline_Phone__c 
FROM Lead WHERE (isConverted = FALSE AND Check_Lead__c = TRUE])
AND (Company != null or Postcode__c != null or Landline_Phone__c != null))]

Just a thought...

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .