Code efficiency help: Can't avoid CPU limit (even with batch)

Question

I'm at my wits end guys. I've rewritten my code a half dozen times now. First I though I could increase efficiency with my loops and if clauses, then I started incorporating tips I read using maps (building maps directly from SOQL queries, etc), I tried using asynchronous apex (@future notation). Now I've recreated my work to be batchable, and set my scope to a single record. My test has been executing for 36 minutes now with no results returned

A brief outline of the goal of the program: I have a custom object (called Client Metrics), and each record represents a unique combination of month, product, and account. These records are populated from contract information, using the start and end dates of the contracts, as well as the products included in the contract. For example, a 12 month contract for two products should generate 24 Client Metric records.

An additional requirement: We often have contracts for the same account/product whose active date periods begin/end within the same month (ie first contract ends January 5, second contract starts January 20). In those cases, only one Client Metric record should be created for that month, so each month/product/account combo is unique.

With all that said, here's the method my Batch Class' execute function is calling, using a SOQL query on all contracts, and with the scope set as 1 contract record:

static public void ContractHandler(list<contract> cList){

    Contract c = cList[0];
    list<Client_Metrics__c> cmList = new list<Client_Metrics__c>();
    list<string> products = c.Product_Content_to_be_Licensed__c.split(';');
    date monthDate;

    date startMonthDate = c.StartDate.toStartOfMonth();
    date endMonthDate = c.EndDate.toStartOfMonth();

    for(string product : products){
        monthDate = startMonthDate;

        list<Client_Metrics__c> startDupes = [SELECT ID, Month_Date__c, Active_Days_in_Month__c, Account__c, Product__c 
                                              FROM Client_Metrics__c
                                              WHERE (Month_Date__c = :startMonthDate)
                                                    AND Account__c = :c.AccountID 
                                                    AND Product__c = :product];

        if(startDupes.isEmpty()){
            cmList.add(new Client_Metrics__c(Month_Date__c = monthDate,
                                             Account__c = c.AccountId,
                                             Product__c = product,
                                             Active_Days_in_Month__c = (date.daysInMonth(monthDate.year(), monthDate.month()) - c.StartDate.day() + 1) / date.daysInMonth(monthDate.year(), monthDate.month())));
        }
        else{
            startDupes[0].Active_Days_in_Month__c = startDupes[0].Active_Days_in_Month__c + ((date.daysInMonth(monthDate.year(), monthDate.month()) - c.StartDate.day() + 1) / date.daysInMonth(monthDate.year(), monthDate.month()));
            cmList.add(startDupes[0]);
        }

        monthDate.addMonths(1);

        while(monthDate < endMonthDate){

            cmList.add(new Client_Metrics__c(Month_Date__c = monthDate,
                                             Account__c = c.AccountId,
                                             Product__c = product));
            monthDate.addMonths(1);             
        }

        list<Client_Metrics__c> endDupes = [SELECT ID, Month_Date__c, Active_Days_in_Month__c, Account__c, Product__c 
                                              FROM Client_Metrics__c
                                              WHERE (Month_Date__c = :endMonthDate)
                                                    AND Account__c = :c.AccountID 
                                                    AND Product__c = :product];

        if(endDupes.isEmpty()){
            cmList.add(new Client_Metrics__c(Month_Date__c = monthDate,
                                             Account__c = c.AccountId,
                                             Product__c = product,
                                             Active_Days_in_Month__c = (c.EndDate.day() / date.daysInMonth(monthDate.year(), monthDate.month()))));
        }
        else{
            endDupes[0].Active_Days_in_Month__c = endDupes[0].Active_Days_in_Month__c + (c.EndDate.day() / date.daysInMonth(monthDate.year(), monthDate.month()));
            cmList.add(endDupes[0]);
        }

    }

    upsert cmList;
}

The top level for loop in the method loops through the products listed in the contract. The 'startDupes' and 'endDupes' SOQL queries are to check if a previous batch already created a record for the month/product/account combo. Since this will only happen on the tail ends of the contract period, I handle the first and final month of the contract outside of the while loop, which handles all the months in between.

And though its not particularly relevant, "Active_Days_in_Month__c" is for those first/final months where the contract starts partway through the month. The field represents the percentage of days out of that month that are part of an active contract period (and when two contracts fall in the same months, their respective percentages are combined).

OK guys, sorry for the wall of text, but hopefully I've explained everything. I've been working so long on optimizing this bad boy that I can barely look at it anymore. ANY insight at all into what I could be doing better will have my heartfelt thanks, and anything else I can do for you!

score 3 · Accepted Answer · edited Apr 13 '17 at 12:54

3

One problem is this which can easily stay in an infinite loop:

    monthDate.addMonths(1);

    while(monthDate < endMonthDate){

        cmList.add(new Client_Metrics__c(Month_Date__c = monthDate,
                                         Account__c = c.AccountId,
                                         Product__c = product));
        monthDate.addMonths(1);             
    }

which you probably meant to be this:

    monthDate = monthDate.addMonths(1);

    while(monthDate < endMonthDate){

        cmList.add(new Client_Metrics__c(Month_Date__c = monthDate,
                                         Account__c = c.AccountId,
                                         Product__c = product));
        monthDate = monthDate.addMonths(1);             
    }

though I would have expected your code to hit a governor limit rather than just stay executing.

The Date object is immutable so a new Date object has to be returned to change the value and your code needs to use the new value.

Adding System.debug statements to check that what you think the code is doing is what it is actually doing can help in situations like this. See e.g. How do I start to debug my own Apex code? for more information on how to do that.

edited Apr 13 '17 at 12:54

Community

1

answered Aug 26 '14 at 21:46

Keith C

135,775
26
201
437

@smoshyee, and also 2 SQL queries into a loop is not really best practice. Not sure how many products you have but you must put them out of the for loop. – brovasi Aug 26 '14 at 23:20
@brovasi Probably more of an "ideally" or "preferably" than a "must" if the number of products is guaranteed to be less than 50? – Keith C Aug 26 '14 at 23:34
@brovasi I know what you mean, and my original unbatched code definitely didn't have queries in loops. But with this I have the scope set to a single contract, so there are guaranteed to be only two queries per product, of which there are a max of five on a single contract. – smohyee Aug 27 '14 at 00:54
@KeithC Haha wow, right on, my test passed. Kinda wish it was more epic than that given how much I've been stressing over it lol. – smohyee Aug 27 '14 at 01:03
@smohyee All it takes is one line of code to be wrong - I've been there too. Getting some new information usually helps; in Java you'd use a debugger but in Apex System.debug is usually easiest. – Keith C Aug 27 '14 at 07:15

Code efficiency help: Can't avoid CPU limit (even with batch)

1 Answers1