Sign Up ›

Browse by Topic
ShowAll Questionssorted byDate Posted
+ Start a Discussion
SeanCenoSeanCeno 

Sales Summary - Apex CPU Time Limit Exceed

All,
I need to make my sales summary code more efficient. Once we hit 9,000 trades on a single Account, the system hits the CPU Limit governor limit, even in a batch job. Anybody have any ideas on the code below? Perhaps moving each calculation in the For loop under each  corresponding map? I'm not sure the best way to change this code, but any help would be greatly appreciated.

Debug log shows errors at random at lines 73, 79, or 92 in the Class and every time on line 4 in the trigger.
CPU Time varies on the number of trades inserted at a time, but even at 1 trade I can get up to 25000 out of 10000 (more than double).

Class: 
public class Account_RollupTrades {
    public Static Account_Setting__c setting = Account_Setting__c.getInstance();
    public Static boolean inprog = false;

    public static void execute (Set<Id> accountIds, List<Account> accountsList) {
        Map<Id, Account> accounts = new Map<Id, Account> (AccountsList);
        system.debug ('execute');
        if(setting.Disable_RollupTrades__c != true) {
            //Map<Id, Account> accounts = new Map<Id, Account>();
            for(Id accountId:accountIds) {
                system.debug(accountid);
                accounts.put(accountId,
                   new Account(
                       Id=accountId,
                       /**YTD_NIOR_I_Sales__c = 0,         YTD_NIOR_I_Shares__c = 0,         QTD_NIOR_I_Sales__c = 0,         QTD_NIOR_I_Shares__c = 0,
                       MTD_NIOR_I_Sales__c = 0,         MTD_NIOR_I_Shares__c = 0,         PY_NIOR_I_Sales__c = 0,          PY_NIOR_I_Shares__c = 0,
                       Total_NIOR_I_Sales__c = 0,       Total_NIOR_I_Shares__c = 0,       YTD_NS_Income_Sales__c = 0,      YTD_NS_Income_Shares__c = 0,
                       QTD_NS_Income_Sales__c = 0,      QTD_NS_Income_Shares__c = 0,      MTD_NS_Income_Sales__c = 0,      MTD_NS_Income_Shares__c = 0,
                       PY_NS_Income_Sales__c = 0,       PY_NS_Income_Shares__c = 0,       Total_NS_Income_Sales__c = 0,    Total_NS_Income_Shares__c = 0,**/
                       Total_NS_HI_Sales__c = 0,       Total_NS_HI_Shares__c = 0,       YTD_NS_HI_Sales__c = 0,         YTD_NS_HI_Shares__c = 0,
                       QTD_NS_HI_Sales__c = 0,         QTD_NS_HI_Shares__c = 0,         MTD_NS_HI_Sales__c = 0,         MTD_NS_HI_Shares__c = 0,
                       PY_NS_HI_Sales__c = 0,          PY_NS_HI_Shares__c = 0,          Total_NS_Income_II_Sales__c = 0, Total_NS_Income_II_Shares__c = 0,
                       YTD_NS_Income_II_Sales__c = 0,   YTD_NS_Income_II_Shares__c = 0,   QTD_NS_Income_II_Sales__c = 0,   QTD_NS_Income_II_Shares__c = 0,
                       MTD_NS_Income_II_Sales__c = 0,   MTD_NS_Income_II_Shares__c = 0,   PY_NS_Income_II_Sales__c = 0,    PY_NS_Income_II_Shares__c = 0,
                       Rollup_Trades__c = DateTime.now()
                   )
                            );
            }
            Map<String, SObjectField[]>
                ytd = new map<string, sobjectfield[]> {
                    //'3910' => new sobjectfield[] { account.YTD_NIOR_I_Sales__c , account.YTD_NIOR_I_Shares__c},
                    //'3911' => new sobjectfield[] { account.YTD_NS_Income_Sales__c , account.YTD_NS_Income_Shares__c },
                    '3912' => new sobjectfield[] { account.YTD_NS_HI_Sales__c , account.YTD_NS_HI_Shares__c },
                    '3915' => new sobjectfield[] { account.YTD_NS_Income_II_Sales__c , account.YTD_NS_Income_II_Shares__c }    
                },
                qtd = new map<string, sobjectfield[]> {
                    //'3910' => new sobjectfield[] { account.QTD_NIOR_I_Sales__c , account.QTD_NIOR_I_Shares__c},
                    //'3911' => new sobjectfield[] { account.QTD_NS_Income_Sales__c , account.QTD_NS_Income_Shares__c },
                    '3912' => new sobjectfield[] { account.QTD_NS_HI_Sales__c , account.QTD_NS_HI_Shares__c },
                    '3915' => new sobjectfield[] { account.QTD_NS_Income_II_Sales__c , account.QTD_NS_Income_II_Shares__c }
                },
                mtd = new map<string, sobjectfield[]> {
                    //'3910' => new sobjectfield[] { account.MTD_NIOR_I_Sales__c , account.MTD_NIOR_I_Shares__c},
                    //'3911' => new sobjectfield[] { account.MTD_NS_Income_Sales__c , account.MTD_NS_Income_Shares__c },
                    '3912' => new sobjectfield[] { account.MTD_NS_HI_Sales__c , account.MTD_NS_HI_Shares__c },
                    '3915' => new sobjectfield[] { account.MTD_NS_Income_II_Sales__c , account.MTD_NS_Income_II_Shares__c }
                },
                py = new map<string, sobjectfield[]> {
                    //'3910' => new sobjectfield[] { account.PY_NIOR_I_Sales__c , account.PY_NIOR_I_Shares__c},
                    //'3911' => new sobjectfield[] { account.PY_NS_Income_Sales__c , account.PY_NS_Income_Shares__c },
                    '3912' => new sobjectfield[] { account.PY_NS_HI_Sales__c , account.PY_NS_HI_Shares__c },
                    '3915' => new sobjectfield[] { account.PY_NS_Income_II_Sales__c , account.PY_NS_Income_II_Shares__c }
                },
                total = new map<string, sobjectfield[]> {
                    //'3910' => new sobjectfield[] { account.Total_NIOR_I_Sales__c , account.Total_NIOR_I_Shares__c},
                    //'3911' => new sobjectfield[] { account.Total_NS_Income_Sales__c , account.Total_NS_Income_Shares__c },
                    '3912' => new sobjectfield[] { account.Total_NS_HI_Sales__c , account.Total_NS_HI_Shares__c },
                    '3915' => new sobjectfield[] { account.Total_NS_Income_II_Sales__c , account.Total_NS_Income_II_Shares__c }
                };
    
    // We make the select in a "for" so instead of selecting to many records at once hitting the heap size limit the for it will take only 200 records to work with at each iteration.
    for(Trades__c[] tradesList : [select Dollar_Amount_of_the_transaction__c, Fund_Number__c, Number_of_Shares_of_the_transaction__c, Resolved_Firm_Trading_ID__c, Resolved_Firm_Trading_IDs__c, Trade_Date__c from Trades__c where Resolved_Firm_Trading_ID__c in :accountIds and Fund_Number__c in (/**'3910', '3911',**/ '3912', '3915') and Dollar_Amount_of_the_transaction__c != null and Number_of_Shares_of_the_transaction__c != null and Trade_Date__c != null and Dollar_Amount_of_the_transaction__c >= 0 and Number_of_Shares_of_the_transaction__c >= 0 FOR UPDATE]){
        for(trades__c trade: tradesList) {
            
            if(date.today().year() == trade.trade_date__c.year()) {
                accounts.get(trade.Resolved_Firm_Trading_ID__c).put(ytd.get(trade.fund_number__c)[0], ((Decimal)accounts.get(trade.Resolved_Firm_Trading_ID__c).get(ytd.get(trade.fund_number__c)[0]))+trade.Dollar_Amount_of_The_Transaction__c);
                accounts.get(trade.Resolved_Firm_Trading_ID__c).put(ytd.get(trade.fund_number__c)[1], ((Decimal)accounts.get(trade.Resolved_Firm_Trading_ID__c).get(ytd.get(trade.fund_number__c)[1]))+trade.Number_of_Shares_of_the_transaction__c);
                //system.debug(ytd);
                //system.debug(LoggingLevel.DEBUG, + Limits.getCpuTime() + '/' + Limits.getLimitCpuTime());
                
                //( (date.ValueOf(date.today().month()).divide(3, 0) == date.ValueOf(trade.trade_date__c.month()).divide(3, 0)) )
                if((((date.today().month() - 1) / 3) + 1) == (((trade.Trade_Date__c.month() - 1) / 3) + 1))   {
                    accounts.get(trade.Resolved_Firm_Trading_ID__c).put(qtd.get(trade.fund_number__c)[0], ((Decimal)accounts.get(trade.Resolved_Firm_Trading_ID__c).get(qtd.get(trade.fund_number__c)[0]))+trade.Dollar_Amount_of_The_Transaction__c);
                    accounts.get(trade.Resolved_Firm_Trading_ID__c).put(qtd.get(trade.fund_number__c)[1], ((Decimal)accounts.get(trade.Resolved_Firm_Trading_ID__c).get(qtd.get(trade.fund_number__c)[1]))+trade.Number_of_Shares_of_the_transaction__c);
                    //system.debug(qtd);
                    //system.debug(LoggingLevel.DEBUG, + Limits.getCpuTime() + '/' + Limits.getLimitCpuTime());
                    
                    if(date.today().month()==trade.trade_date__c.month()) {
                        accounts.get(trade.Resolved_Firm_Trading_ID__c).put(mtd.get(trade.fund_number__c)[0], ((Decimal)accounts.get(trade.Resolved_Firm_Trading_ID__c).get(mtd.get(trade.fund_number__c)[0]))+trade.Dollar_Amount_of_The_Transaction__c);
                        accounts.get(trade.Resolved_Firm_Trading_ID__c).put(mtd.get(trade.fund_number__c)[1], ((Decimal)accounts.get(trade.Resolved_Firm_Trading_ID__c).get(mtd.get(trade.fund_number__c)[1]))+trade.Number_of_Shares_of_the_transaction__c);
                        //system.debug(mtd);
                        //system.debug(LoggingLevel.DEBUG, + Limits.getCpuTime() + '/' + Limits.getLimitCpuTime());
                    }
                }
            } 
            else if(date.today().year()-1==trade.trade_date__c.year()) {
                accounts.get(trade.Resolved_Firm_Trading_ID__c).put(py.get(trade.fund_number__c)[0], ((Decimal)accounts.get(trade.Resolved_Firm_Trading_ID__c).get(py.get(trade.fund_number__c)[0]))+trade.Dollar_Amount_of_The_Transaction__c);
                accounts.get(trade.Resolved_Firm_Trading_ID__c).put(py.get(trade.fund_number__c)[1], ((Decimal)accounts.get(trade.Resolved_Firm_Trading_ID__c).get(py.get(trade.fund_number__c)[1]))+trade.Number_of_Shares_of_the_transaction__c);
                //system.debug(py);
                //system.debug(LoggingLevel.DEBUG, + Limits.getCpuTime() + '/' + Limits.getLimitCpuTime());
            }
            accounts.get(trade.Resolved_Firm_Trading_ID__c).put(total.get(trade.fund_number__c)[0], ((Decimal)accounts.get(trade.Resolved_Firm_Trading_ID__c).get(total.get(trade.fund_number__c)[0]))+trade.Dollar_Amount_of_The_Transaction__c);
            accounts.get(trade.Resolved_Firm_Trading_ID__c).put(total.get(trade.fund_number__c)[1], ((Decimal)accounts.get(trade.Resolved_Firm_Trading_ID__c).get(total.get(trade.fund_number__c)[1]))+trade.Number_of_Shares_of_the_transaction__c);
            //system.debug(total);
            //system.debug(LoggingLevel.DEBUG, + Limits.getCpuTime() + '/' + Limits.getLimitCpuTime());
        }
            }
        }
        inprog = true;
        update accounts.values();
        inprog = false;
    }
}
Trigger: 
trigger Account_RollupTrades on Account (after update) {
    if(Account_RollupTrades.inprog == false) {
        //set<ID> sID = new set<ID> (trigger.newMap.keySet());
        Account_RollupTrades.execute(trigger.newMap.keySet(), trigger.new);
    }
}



 
Here-n-nowHere-n-now
Do you have bulk updates going on with your accounts?  Basically the computing time of your logic is on the O(a*t) level, i.e., proportional to the product of number of accounts and the number of trades, which could increase pretty fast if you have bulk updates, and/or many trades per account.  The other thing I would look into is map operations.  I'm not sure how efficient the Apex maps are.  Your logic is fairly simple arithmatic plus a lot of map operations (up to 60+ per trade per account), which could be bad if the Apex map isn't that efficient.  You had a lot of debugging outputs for CpuTime - what did you get when you timed each section of the calculation?
SeanCenoSeanCeno
Yes, the batch job runs through every Account every morning. And if an account has more than 9000 trades in 1 fund (we only have 2 accounts that do right now), it will fail with a CPU time of anywhere around 25000 out of the 10000 allowed. What I'm looking for is help in writing the code to be more efficient or a different solution. I tried to define the Batchable Class to only do 5 Accounts in each batch, but it still fails. Any help on reformatting my code would be greatly appreciated!
 
Here-n-nowHere-n-now
I think it's the way you did the calculations - using get() and put() calls on sObject records.  Those are less the straight assignment syntax, e.g. myAcct.my_field__c + = 100. I did a quick comparison and here's the data.  For a 900-record loop, if I do straight assignment, it's about 48 ms.  If I do get/put, it's 77 ms.  My test code below (debug printout at the bottom).  So if you have 9000-record, and your code does much more get/put calls (I counted 40+ per iteration), the difference would be well above 6000 ms (assuming code runs equally fast regardless instance/org).  Does that make sense?

My Test code
______________________________________________________________________
    static testMethod void myUnitTest() {
        //
        Account [] accts = [Select Id, Total_Referred_Customers__c From Account Where Recordtype.name ='Partner Group' and Total_Referred_Customers__c != null Limit 900];
        
        
        system.debug(LoggingLevel.ERROR, 'start: ' + Limits.getCpuTime() + '/' + Limits.getLimitCpuTime());
        for (Account a : accts) {
            a.put('Total_Referred_Customers__c',(Decimal)a.get('Total_Referred_Customers__c') + 1);
        }
        
        system.debug(LoggingLevel.ERROR, 'after get/put: ' + Limits.getCpuTime() + '/' + Limits.getLimitCpuTime());
        for (Account a : accts) {
            a.Total_Referred_Customers__c +=  1;
        }
        
        system.debug(LoggingLevel.ERROR, 'after plain ops: ' + Limits.getCpuTime() + '/' + Limits.getLimitCpuTime());

    }
______________________________________
Debug printout
______________________________________
16:24:08.337 (22337863632)|USER_DEBUG|[30]|ERROR|start: 17/10000
16:24:08.424 (22424260707)|USER_DEBUG|[35]|ERROR|after get/put: 94/10000
16:24:08.477 (22477407381)|USER_DEBUG|[40]|ERROR|after plain ops: 142/10000
Here-n-nowHere-n-now
You probably would still want to do this in a batchable class instead of a trigger (even with refactoring to simple assignment) - it gives you 60000 ms to work with instead of 10000, which would make your life easier in terms of how fast you can process the whole thing, because you can process records in bigger batches.
SeanCenoSeanCeno
So yes, that does make sense as to the length of time it takes, but I'm not sure how to remedy it. I thought having a trigger was required when trying to update fields? And I have this in a batch class, how do I still update the fields without the trigger though?

This code set below inserts the trades in a batch job every morning. The code above posts it to the proper Account for a sales summary. 
public class Trades_CascadeAccounts {
    public Trades__c[] tradesOldList { set; get; }
    public Trades__c[] tradesNewList { set; get; }
    public Map<Id, Trades__c> tradesOldListMap { set; get; }
    
    public Trades_CascadeAccounts(Trades__c[] tradesOldList, Trades__c[] tradesNewList) {
        this.tradesNewList = tradesNewList == null ? new Trades__c[0] : tradesNewList.clone();
        this.tradesOldList = tradesOldList == null ? new Trades__c[0] : tradesOldList.clone();
        this.tradesOldListMap = new Map<Id, Trades__c>(this.tradesOldList);
    }
    
    public void execute() {
        Set<Id> tradesAccountIds = new Set<Id> {};
            for(Trades__c tradesNew : tradesNewList) {
                Trades__c tradesOld = tradesOldListMap.containsKey(tradesNew.Id) ? tradesOldListMap.get(tradesNew.Id) : new Trades__c();
                tradesAccountIds.addAll(new Set<Id> { tradesOld.Resolved_Firm_Trading_ID__c, tradesNew.Resolved_Firm_Trading_ID__c });
            }
        tradesAccountIds.remove(null);
        
        Account[] accountList = new Account[0];
        for(Id accountId : tradesAccountIds) {
            accountList.add(new Account(Id=AccountId,Rollup_Trades__c=null));
        }
        update accountList;
        //System.debug('Total Number of script statements allowed in this apex code context: ' +  Limits.getLimitScriptStatements());  
    }
}
  
trigger Trades_CascadeAccounts on Trades__c (after delete, after insert, after undelete, after update) {
    Trades__c[] tradesOldList = trigger.IsDelete ? null : trigger.old;
    Trades__c[] tradesNewList = trigger.IsDelete ? trigger.old : trigger.new;
    new Trades_CascadeAccounts(tradesOldList, tradesNewList).execute();
}
  
global class Trades_CascadeAccountsBatchable implements Database.Batchable<sObject>{
    global Database.QueryLocator start(Database.BatchableContext batchableContext){
        return Database.getQueryLocator('select Id from Account where ParentId = null');
    }
    global void execute(Database.BatchableContext batchableContext, Account[] accountList) {
        for(Account account : accountList)
            account.Rollup_Trades__c = null;
        update accountList;
    }
    global void finish(Database.BatchableContext batchableContext) {
    }
}
  
global class Trades_CascadeAccountsSchedulable implements Schedulable {
    global void execute(SchedulableContext schedulableContext) {
        Database.executeBatch(new Trades_CascadeAccountsBatchable(), 5);
    }
}

 
Here-n-nowHere-n-now
You can certainly update fields without triggers - basically you do all that in the execute() call in your batchable class.  However in this case (based on the code you posted), you're already using batchable to do account updates, which in turn does the calculation in the account trigger, the whole context is considered asynchronous and thus enjoy the higher limit, so there's no real need to refactor and put everything in batchable.  So the key now is to refactor the account trigger to use simple assignments instead of sObject put() and get().  That should go a long way to reduce CPU time usage.

Also the Trades__c trigger is also in use (as posted)?  Are you trying to recalc after each trade updates?  I'd suggest disabling if that's not necessary, because it's a synchronous context, so you have a lower limit for CPU time.
Vijaya Kumar RegantiVijaya Kumar Reganti
Hi,

can you try using the @future annotation for the method once??

Because the @future increases the CPU time limit from normal 10 sec to 60 sec.

Thanks,
Vijay
SeanCenoSeanCeno
Here-n-now,
Thanks, I will certainly work on this and try to optimize my code for a lower CPU time.

Vijaya,
I've actually never used a @future annotation. I tried adding it to line 4 before the "public static void execute" of the class, but it's throwing the error: 
Unsupported parameter type List<Account>

Am I missing something?
Here-n-nowHere-n-now
With @future annotation you can't pass sObject parameters.  Primitives are ok, including IDs, so you can pass a list of account IDs.
SeanCenoSeanCeno
Hey Here-n-now,

I'm finally able to revisit this and I think if I change the calculations to straight assignments, I might be alright. How would I go about converting the get/puts to straight assignments? Do I have to write a SOQL for each one with a where clause for YTD, MTD, QTD, etc?