InventABCUpdate - Very Poor Performance

Recently, our purchasing manager tried to run the InventABCUpdate batch process in our QA environment using the parameters defined below.

 

 

The process took 14 hours to run and all it was doing was trying to identify which ABC code should be assigned to each item in the InventTable (not service items). 

 

In basic terms, what this routine trying to calculate which items contributed to the first 80% of revenue (value 'A'), the items in the band from 80% to 95% of revenue (value 'B') and the remainder (value 'C').  This is a general classification that does not take into account any kind of quantity.  So an item that sold for $20,000.00 during this period and is only sold occasionally would have the same code as an item that had a purchase price of $1,000.00 and sold 20 units during the period.  So, if total sales were $1,000,000.00 during the period, the ABC values would be assigned in the following manner.

 

For each item order by total sales for the period, if it occurs in the band from $0.00 to $800,000.00 incrementing by the sales for the period, it will be assigned a value of A.  If occurs from $800,000.00 to $950,000.00 it will be assigned a B.  Otherwise an item will be assigned a C.

Item Id	Sales for Period	Accumulated Sales	ABC Value
AAA	100,000	100,000	A
BBB	99,000	199,000	A
CCC	98,000	197,000	A
DDD	97,000	194,000	A
...
QQQ	54,000	799,000	A
RRR	53,000	852,000	B
SSS	52,000	904,000	B
...
XXX	3,000	994,000	B
YYY	2,000	996,000	B
ZZZ	1,500	997,500	C
...

 

So why was this job taking 14 hours?  The job begins with InventABCUpdate that processes the parameters, calls the factory method newFromInventABCUpdate to create the correct subclass of InventABC and passes the results to the Calc method in InventABC.  In this case, the subclass instantiated is InventABC_Revenue.

The Calc method has 2 parts, the first is to populate to the TmpABC table to by calling the subclasses’ sumUpValue method.  The second part iterates through each TmpABC record ordered by Amount descending.  Based on the discussion earlier, it derives the correct ABC value, retrieves the InventTable record buffer, assigns the value and updates it.  In our case, we have some 177,000 records in the InventTable involved, so this should take some time but not hours.  Actually, it is about a minute.

The real culprit is the sumUpValue method in the InventABC_Revenue class. The method as written is:

protected void sumUpValue()

{

    InventTable             inventTable;

    InventTable             previousInventTable;

    CustInvoiceTrans        custInvoiceTrans; 

    Query    query = new Query(itemQueryRun.query().pack());

    QueryBuildDataSource    qbdsItem = query.dataSourceTable(tableNum(InventTable));

    QueryBuildDataSource    qbdsTrans = qbdsItem.addDataSource(tableNum(CustInvoiceTrans));

    QueryBuildRange         itemTypeRange = SysQuery::findOrCreateRange(qbdsItem, fieldNum(InventTable, ItemType));

    QueryRun                revenueQueryRun;

    AmountMST               lineAmountMST;

 

    // ensure that results are sorted by item

    qbdsItem.addOrderByField(fieldNum(InventTable, ItemId));

    itemTypeRange.value(SysQuery::valueNot(queryValue(ItemType::Service)));

    qbdsTrans.relations(false);

    qbdsTrans.addLink(fieldNum(InventTable, ItemId), fieldNum(CustInvoiceTrans, ItemId));

    qbdsTrans.joinMode(JoinMode::OuterJoin);

    // filter on dates

    qbdsTrans.addRange(fieldNum(CustInvoiceTrans, InvoiceDate)).value(SysQuery::range(fromDate, toDate));

    revenueQueryRun = new QueryRun(query);

    while (revenueQueryRun.next())

    {

        if (revenueQueryRun.changed(tableNum(InventTable)))

        {

            inventTable = revenueQueryRun.get(tableNum(InventTable)) as InventTable;

            if (previousInventTable.ItemId)

            {

                this.saveAmount(previousInventTable, lineAmountMST);

                lineAmountMST = 0;

            }

            previousInventTable.data(inventTable);

        }

 

        if (revenueQueryRun.changed(tableNum(CustInvoiceTrans)))

        {

            custInvoiceTrans = revenueQueryRun.get(tableNum(CustInvoiceTrans)) as CustInvoiceTrans;

            if (custInvoiceTrans.LineAmountMST)

            {

                lineAmountMST += custInvoiceTrans.LineAmountMST;

            }

        }

    }

    if (previousInventTable.ItemId)

    {

        this.saveAmount(previousInventTable, lineAmountMST);

    }

}

 

The problem is this method is the query.  What it is doing is iterating through each InventTable and each associated record (if any) in CustInvoiceTrans to accumulate its total sales.  On top of this, it is ordering this query by item id.  According to the debugger, the query is written as:

SELECT FORUPDATE * FROM InventTable(InventTable) ORDER BY InventTable.ItemId ASC WHERE ((NOT (ItemType = 2))) OUTER JOIN * FROM CustInvoiceTrans(CustInvoiceTrans_1) ON InventTable.ItemId = CustInvoiceTrans.ItemId AND ((InvoiceDate>={ts '2015-09-01 00:00:00.000'} AND InvoiceDate<={ts '2016-08-31 00:00:00.000'}))

This translates into SQL as:

SELECT * FROM InventTable

LEFT OUTER JOIN CustInvoiceTrans ON InventTable.ItemId = CustInvoiceTrans.ItemId

AND ((InvoiceDate>={ts '2015-09-01 00:00:00.000'}

AND InvoiceDate<={ts '2016-09-01 00:00:00.000'}))

WHERE ((NOT (ItemType = 2)))

 

In our database, this totaled 959, 472 records and took 2 minutes and 48 seconds to run.

A more direct query:

SELECT IT.ITEMID, SUM(CIT.LINEAMOUNTMST) LINEAMOUNTMST

FROOM INVENTTABLE IT

INNER JOIN CUSTINVOICETRANS CIT ON IT.ITEMID = CIT.ITEMID

       --AND IT.DATAAREAID = CIT.DATAAREAID

       --AND IT.PARTITION = CIT.PARTITION

WHERE CIT.INVOICEDATE >= '2015-09-01'

AND CIT.INVOICEDATE <= '2016-09-01'

GROUP BY IT.ITEMID

ORDER BY IT.ITEMID

Returned 77,003 records and took 4 seconds to run.  It also has 12.4x fewer records, thus speeding up the additional processing after this routine.

It was decided to implement a set of changes based on this processing, but segregated from normal processing so it could be removed if Microsoft implements improvements in this area.  Also, I decided that it would be nice to maintain a record of old ABC Revenue values and new along with the other data used in processing. The record would be access via queries in the database as there was no real need for a UI or report.

Step 1) Create a duplicate tmpABC Table.

 

The only differences are 2 name changes: Amount became TotalSales and Value became NewValue.  The next step was to duplicate InventABC_Revenue.  The only difference here is that the sumUpValue method would be rewritten also a new parameter will be added to InventABC, parmContinueAfterSumUpValue.  At the parent class, it will always return True.  In the new InventABC_Revenue derived class, it will return false.

The new method performs the calculate operation in 4 steps

protected void sumUpValue()

{

    this.initializeABCClassification();

    this.assignRevenueForPeriod();

    this.calculateNewClassification();

    this.updateInventoryTable();

}

InitializeABCClassication deletes all records from the new PC_ABCClassification table.  The reason for this is that a simple merge may have left sales values from prior periods and have returned incorrect results.  It could have been written as update all old ABC values with the current values, new ABC values with ‘C’ and set TotalSales to 0 and then add items created since last run.  However, I was thinking that just refreshing the table would be easier and faster.  After truncating the table, all non-service items are added with the appropriate attributive data.

 

while select ItemId, NameAlias, RecId, ABCRevenue

    from inventTable

        where inventTable.ItemType != ItemType::Service

    join ItemGroupId from inventItemGroupItem where inventItemGroupItem.ItemId == inventTable.ItemId

        && inventItemGroupItem.ItemDataAreaId == inventTable.DataAreaId

    {

 

        pcABCClassification.RefRecId     = inventTable.RecId;

        pcABCClassification.ItemGroupId  = inventItemGroupItem.ItemGroupId;

        pcABCClassification.ItemId       = inventTable.ItemId;

        pcABCClassification.OldValue     = inventTable.ABCRevenue;

        pcABCClassification.ItemName     = inventTable.NameAlias;

        pcABCClassification.NewValue     = ABC::C;

        pcABCClassification.insert();

    }

 

Now that the base data is set, it is now easier to collect the sales data directly.

private void assignRevenueForPeriod()

{

    InventTable inventTable;

    CustInvoiceTrans custInvoiceTrans;

    PC_ABCClassification pcABCClassification;

    ;

 

    while select ItemId,sum(LineAmountMST) from custInvoiceTrans

        group by custInvoiceTrans.ItemId

        where custInvoiceTrans.InvoiceDate >= fromDate

        && custInvoiceTrans.InvoiceDate <= toDate

        && custInvoiceTrans.ItemId

        notexists join inventTable

        where inventTable.ItemId == custInvoiceTrans.ItemId

        && inventTable.ItemType == ItemType::Service

    {

        pcABCClassification = PC_ABCClassification::find(custInvoiceTrans.ItemId, true);

        pcABCClassification.TotalSales = custInvoiceTrans.LineAmountMST;

        pcABCClassification.doUpdate();

    }

 

}

 

This is the method that now traverses only 77.000 records instead of 959,000 records.  By using the group by clause, the subroutine now require 1/12 the processing of before.

Now calculating the new ABC classification is tweaked so that it iterates a little more quickly.  I could have used the old method of classifying, but decided to try this instead.  The only difference is items that have the same sales amount and are on the border between classifications may not get the same code.  This is a highly improbable scenario in our business, so it can safely be ignored.

private void calculateNewClassification()

{

    AmountCur categoryARev;

    AmountCur categoryBRev;

    AmountCur usedAmount = 0;

 

    InventTable inventTable;

    CustInvoiceTrans custInvoiceTrans;

    PC_ABCClassification pcABCClassification;

    ;

    select sum(LineAmountMST) from custInvoiceTrans

        where custInvoiceTrans.InvoiceDate >= fromDate

        && custInvoiceTrans.InvoiceDate < toDate

        && custInvoiceTrans.ItemId

        notexists join ItemId, ItemType from inventTable

        where inventTable.ItemId == custInvoiceTrans.ItemId

        && inventTable.ItemType == ItemType::Service;

 

    totalAmount = custInvoiceTrans.LineAmountMST;

    categoryARev = totalAmount * categoryA / 100;

    categoryBRev = categoryARev + totalAmount * categoryB / 100;

 

    while select forUpdate pcABCClassification

        order by pcABCClassification.TotalSales desc

    {

        if (usedAmount < categoryARev)

        {

            pcABCClassification.NewValue = ABC::A;

        }

        else

        {

            if (usedAmount < categoryBRev)

            {

                pcABCClassification.NewValue = ABC::B;

            }

            else

            {

                pcABCClassification.NewValue = ABC::C;

            }

        }

        pcABCClassification.doUpdate();

        usedAmount = usedAmount + pcABCClassification.TotalSales;

    }

}

 

First, the total sales for the period are determined. With this, the band amounts for A and B classifications are made.  Once the B threshold is passed, then all items regardless of sales volumes (if any) are assigned a value of C.

The final step, iterates through the new PC_ABCClassification table and assigns the NewValue to the ABCRevenue field in the associated InventTableRecord.

private void updateInventoryTable()

{

    InventTable inventTable;

    PC_ABCClassification pcABCClassification;

    ;

    while select forupdate inventTable

        join NewValue from pcABCClassification where pcABCClassification.ItemId == InventTable.ItemId

    {

        this.setCategory(inventTable, pcABCClassification.NewValue);

        inventTable.doUpdate();

    }

}

 

The last steps involved a modification to InventABC method newFromInventABCUpdate to instantiate a new Revenue class object instead of the old version and modify the calc method to exit if parmContinueAAfterSumUpValue is false.

static public InventABC newFromInventABCUpdate(InventABCUpdate _abcUpdate)

{

    ABCModel    model;

    InventABC   inventABC;

    if (! _abcUpdate)

    {

       throw error(strFmt("@SYS22533",classId2Name(classNum(InventABC)),classId2Name(classNum(InventABCUpdate))));

    }

    model = _abcUpdate.parmModel();

 

    switch (model)

    {

        case ABCModel::Revenue  :

 

            //inventABC = InventABC_Revenue::construct();

            /*

                DG: Override current behavior with new class.

            */

            inventABC = PC_InventABC_Revenue::construct();

            break;

        case ABCModel::ContributionMargin:

            inventABC = InventABC_ContributionMargin::construct();

            break;

        case ABCModel::Value:

            inventABC = InventABC_InventValue::construct();

            break;

        case ABCModel::Link   :

            inventABC = InventABC_CarrCost::construct();

            break;

        default:

            throw error(strFmt("@SYS22532",model));

    }

    inventABC.initFromInventABCUpdate(_abcUpdate);

    return inventABC;

}

 

public void calc()

{

    InventTable     inventTable;

    Amount          accAmount;

    Amount          lastAmount;

    Percent         proportion;

    Percent         proportionBefore;

    ABC             category;

    ABC             lastCategory;

 

    setPrefix("@SYS19424");

 

    this.sumUpValue();

 

    /*

    DG: In order to make this routine more efficient, the assignment of

    new abc codes for the PC_InvenABC_Revenue class was done internally.

    */

    if (!this.parmContinueAfterSumUpValue())

        return;

 

    if (! totalAmount)

    {

        throw error(strFmt("@SYS22534",CompanyInfo::standardCurrency()));

    }

…

 

Total run time is now about 3-5 minutes.