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.



Bulk Importing XPOs

Recently, we had a consultant do a large set of fixes to our system.  As part of the agreement, he was supposed to implement best practice fixes across the system.  Given the number of places this needed to be touched, it was a bad idea.  However, the decision was not in my hands.

Once the changes were migrated to our Test environment, we started our regression tests.  Everything went well until we got to printing customer invoices.  No matter what we tried, we couldn’t get that piece to work.  As a result, management decided to rollback everything.  The problem was that in the intervening time a large number of other changes had been promoted to build.  As a result, we needed to restore our model store to the state the consultant’s changes had been checked into TFS.  Then each change after the consultant’s needed to be integrated into the original code base.  It was painful, but I finished it in a couple of weeks.

Now I had a collection of XPOs that needed to be turned into a new model store.  As a baseline, I simply restored a backup model store from production and then disconnected build from TFS.  In order to import the corrected XPOs, I decided to build a tool to help automate the imports.  After the imports, I attached build to a new TFS project and exported the model store.

The tool itself was a little unique.  Given that this just a quick and dirty exercise, I decided to write it as a windows form.  The final form was quite simple:

In order to perform the operation, the user needed to select the root directory where all the XPOs are located.  Once selected, all XPO files in the selected directory and its subdirectories are found and listed in the textbox.

private void btnRootDirectory_Click(object sender, EventArgs e)

{

FolderBrowserDialog fbd = new FolderBrowserDialog();

lstXPOs.DataSource = null;

 

fbd.RootFolder = Environment.SpecialFolder.MyComputer;

if(fbd.ShowDialog() == DialogResult.OK)

{

txtRootDirectory.Text = fbd.SelectedPath;

lstXPOs.DataSource = Directory.GetFiles(txtRootDirectory.Text,"*.xpo", SearchOption.AllDirectories);

}

}

 

 

Next, the user needs to select an output directory.  This just calls a FolderBrowserDialog.

private void btnFindOutputDirectory_Click(object sender, EventArgs e)

{

FolderBrowserDialog fbd = new FolderBrowserDialog();

 

fbd.RootFolder = Environment.SpecialFolder.MyComputer;

if (fbd.ShowDialog() == DialogResult.OK)

{

txtOutputDirectory.Text = fbd.SelectedPath;

}

}

 

 

The output of the process button will be an xml file detailing which files to import and a directive to import without compiling and a bat file to begin the importing process. 

private void bntProcess_Click(object sender, EventArgs e)

{

if (lstXPOs.DataSource != null && !string.IsNullOrWhiteSpace(txtRootDirectory.Text))

{

string[] files = (string[])lstXPOs.DataSource;

string fileToWrite = txtOutputDirectory.Text + @"\aximport.xml";

string batchToWrite = txtOutputDirectory.Text + @"\aximport.bat";

string logFile = txtOutputDirectory.Text + @"\aximport.log";

if (File.Exists(fileToWrite))

{

File.Delete(fileToWrite);

              }

using (StreamWriter sw = new StreamWriter(fileToWrite))

{

sw.WriteLine("<?xml version=\"1.0\" encoding=\"utf-8\"?>");

sw.WriteLine("<AxaptaAutoRun exitWhenDone=\"true\" version=\"6.2\" logFile=\"" + logFile + "\" >");

foreach (string fileName in files)

{

string command = "<XpoImport file=\"" + fileName + "\" />";

sw.WriteLine(command);

}

sw.WriteLine("<CompileApplication crossReference=\"false\" />");

sw.WriteLine("</AxaptaAutoRun>");

sw.Close();

}

 

if (File.Exists(batchToWrite))

{

File.Delete(batchToWrite);

}

using (StreamWriter sw = new StreamWriter(batchToWrite))

{

sw.WriteLine("ax32.exe -StartupCmd=AutoRun_" + fileToWrite);

sw.Close();

}

}

}

 

The entire point of the exercise is to generate a command line bat file that will open the Ax client and automatically start importing the designated files.

The bat file contains a single line:

ax32.exe -StartupCmd=AutoRun_C:\AxImportTest\aximport.xml

The import file, is an xml file that contains an autorun instruction and an attribute to designate where the log file should be placed.

<?xml version="1.0" encoding="utf-8"?>

<AxaptaAutoRun exitWhenDone="true" version="6.2" logFile="C:\AxImportTest\aximport.log" >

<XpoImport file="C:\AxImportTest\Parts Canada Model\Classes\MaintainCustomCharges.xpo" />

<XpoImport file="C:\AxImportTest\Parts Canada Model\Classes\SalesPickingListJournalCreate.xpo" />

…

</AxaptaAutoRun>

 

Running the batch file will then import the listed XPO files.