Оптимизация класса Tax

[Lihgt]

Постановка задачи:
На основании модификации, полученной из офиса Колумбуса, изменить механизм расчета налогов при проведении накладной для повышения производительности.

Модификация:
В классе Tax изменен метод adjustAmount

PHP код:

void adjustAmount(ExchRate   _exchRate,
                  ExchRate   _exchrateSecondary)
{
    if (_exchRate)
    {
        ttsbegin;

        while select forupdate taxWorkTrans
        where taxWorkTrans.headingRecId  == headingRecId         &&
              taxWorkTrans.exemptTax     == NoYes::No
            // Оптимизация расчета налогов (c) KURA -->
           && taxWorkTrans.SourceRecId   == this.sourceRecId()
            // Оптимизация расчета налогов (c) KURA <--
        {
 ... 


Что мне известно по этой теме:
Действительно, при расчете налогов по многострочным заказам (закупкам) класс Tax делает очень много пересчетов суммы налогов. Особенно хорошо это видно, если на данную операцию натравить профайлер - Аксапта "уйдет в себя" на продолжительное время. Данная модификация должна решить эту проблему, но есть вопросы.

Вопросы:
1. Какой выигрыш в производительности может дать такое изменение кода?
2. Как это скажется на потомках Tax-а и на расчете "withoutSource"?
1+2 Стоит ли игра свеч, а результат труда (с учетом анализа своих потомков Tax-а)?
3. Кто-дь использует у себя такую оптимизацию?
4. Если все хорошо, то почему ее нет в SP и HF?

P.S. Не знаю как эта тема согласуется с авторскими правами.
P.S.S. Сам еще на занимался плотным анализом механизмов класса Tax и потомков. Поэтому, будет интересна любая инфа по теме.

[mazzy]

Потенциально прирост даст офигительный.

Прежде чем делать стоит углубится в суть проблемы.
Дело в том, что в европе все налоги берутся на общую сумму документа, а не по строкам. Аксапта - европейского происхождения. Поэтому изначально Аксапта (да и Аттейн) не оптимизировались для расчета сумм по каждой строке. Да и алгоритмов таких изначально не было.

Потом пошла экспансия в Америку. Там есть налоги, которые рассчитываются по каждой строке документа. В Аксапту были добавлены соответствующие алгоритмы. НО! Они добавлены. Они не были задуманы там изначально. Со всеми вытекающими последствиями для кода последствиями.

А потом была локализация с НсП по оплате. НсП считается простым повторным пересчетом всех налогов. Со всеми вытекающими для производительности последствиями.

В результате функция расчета налогов в Аксапте есть. Эта функция реализована за счет приемлимых затрат на программирование. Но эта функция неоптимальна.


Теперь к самому вопросу.
Оптимизировать можно (и наверное нужно). Однако я, например, боюсь, что это слишком накладно. Судя по моим ощущениям проще переписать Аксапту, чем подлезть и исправить налоги

Буду рад услышать другие мнения - буду рад ошибаться.

[EVGL]

Mazzy абсолютно прав. Если вы решили пожертвовать возможностью расчета налога со всей, к примеру, накладной, то данная модификация имеет право на существование.

Соответственно, режим Всего, который можно задать в параметрах налогового модуля, станет, скорее всего, недееспособным. На первый взгляд, туда ему и дорога, но пропадает симпатичная возможность скомпенсировать ошибки округления, которые набегают при расчете налога по отдельным строкам.

Насчет НсП: действительно двойной пересчет. Поэтому, если вы не используете НсП, отключите функциональный ключ и сразу получите почти двойной (!) прирост производительности расчета налогов по заказам и накладным на услуги.

[Lihgt]

Повторю задачу:
Проанализировать предложенную оптимизацию на предмет ее применимости в наших условиях.

Что было сделано
1. У заказа был установлен фиксированный курс валюты.
(метод adjustAmount вызывается, когда курс валюты заказа не совпадает с текущим курсом валюты)
2. В класс Tax добавлен счетчик, в котором суммировалось кол-во циклов в while select
3. В форме заказы выбираем пункт меню Запросы-Итоги и смотрим результат

В результате получили:
до оптимизации
1 строка - 2 итерации
2 строки - 8 итераций
3 строки - 18 итераций
4 строки - 32 итерации,
т.е. итераций = 2 * (строк ^ 2)... 100 строк -> 20000 итераций

после оптимизации
1 строка - 2 итерации
2 строки - 4 итерации
3 строки - 6 итераций
4 строки - 8 итераций,
т.е. итераций = 2 * строк ... 100 строк -> 200 итераций
Разница очевидна

А чтобы не жертвовать можно так поменять код

PHP код:

void adjustAmount(ExchRate   _exchRate,
                  ExchRate   _exchrateSecondary)
{
    if (_exchRate)
    {
        ttsbegin;

        while select forupdate taxWorkTrans
        where taxWorkTrans.headingRecId  == headingRecId         &&
              taxWorkTrans.exemptTax     == NoYes::No
            // Оптимизация расчета налогов (c) KURA  & Lihgt -->
           && (taxCalculateTotal || taxWorkTrans.SourceRecId   == this.sourceRecId())
            // Оптимизация расчета налогов (c) KURA  & Lihgt <--
        {
 ... 


P.S. У нас используются только РУБ, следовательно, все эти прелести не для нас

[Logger]

А кто применял данный фикс на реальном проекте ?

Вопрос связан с тем, что сомнения одолели. Даже не верится, что такой маленький фикс решит такую большую проблему. Где-то должен быть подвох.

[fed]

Ладно, поскольку дискуссия с Logger перешла в эту ветку, пожалуй продолжу здесь.

Во первых, в версии 2009 этот метод (adjustAmount) изрядно подоптимизировали. Во вторых - он ничего такого не делает Логика достаточно простая:

1. Он считает сумму в исходной валюте по каждому коду налога по документу (в реальности там группировка сложнее, но в первом приближении это только код налога)
2. Он пересчитывает каждую строку налога в taxTmpWorkTrans - тупым перемножением суммы в валюте на курс. Данная операция выполняется для каждой строки отдельно ( а каждая строка в первом приближении - равно строке закупки или заказа - если у вас только НДС).
3. Сумма в исходной валюте по каждому коду документа пересчитывается по новому курсу в валюту учета. Затем новая сумма размазывается по каждой строке с данным кодом.
4. Данный метод вызывается только один раз.

На самом деле, сложность вычисления равна, грубо говоря n*3, где n-число строк в документе. (Один раз пробежались по n строкам чтобы посчитать суммы по кодам налогов, потом пробежались чтобы пересчитать суммы налога, потом еще раз пробежались чтобы скорректировать).
Однако есть одна загводка: Как известно, временные таблицы хранятся в памяти только если их размер не превышает 128 КБайт. Если размер превысил эту величину, то AOS выгружает их во временный файл, запросами и доступом к которому занимается сам AOS. А внутренний процессор запросов в AOS не быстрый и не оптимизировали его с прошлого столетия, вероятно он часто в fullscan скатывается
Размер одной записи в tmpTaxWorkTrans в стандартной конфигурации, составляет 793 байта. Получается что где-то после 160 строк в заказе/закупке, данные из таблицы выгружаются на диск и начинают страшно тормозить в запросах.

Если в новой версии (2012) tmpTaxWorkTrans переделали в SQL-ную временную таблицу, это должно заметно ускорить процесс. Также можно надеятся, что в 64битной версии сервера (а мне кажется 32битной и не будет) наконец-то отменили ограничение в 128 Кбайт под временную таблицу в памяти и теперь таблица будет храниться в памяти пока таковая имеется.

[Logger]

Хм. Про вытеснение времянок с памяти на диск я и не подумал.
Хотя все равно как-то косячно. Думаю что можно переделать алгоритм так чтобы зависимость от числа строк была линейной.

Если получится - отпишу.

[fed]

Цитата:

Сообщение от Logger

Хм. Про вытеснение времянок с памяти на диск я и не подумал.
Хотя все равно как-то косячно. Думаю что можно переделать алгоритм так чтобы зависимость от числа строк была линейной.

Если получится - отпишу.

Я бы побробовал для начала выгрузить tmptaxWorkTrans в настоящую таблицу с неким guidом в качестве дополнительного поля. Потом прогнал бы по ней все операции, а потом загрузил бы назад в tmpTaxWorkTrans. Изврат конечно - но может и поможет. Особенно если вставлять через recordInsertList.

[Logger]

Ого !
Радикально !

А я думал в мап запихнуть - чтобы гарантировать наличие данные в памяти. Или в RecordSortedList (правда хрен его знает - сбрасывает он данные на диск или нет. Если данных не много - поиск по нему идет примерно с той же скоростью что и по мапам)

[fed]

Цитата:

Сообщение от Logger

Ого !
Радикально !

А я думал в мап запихнуть - чтобы гарантировать наличие данные в памяти. Или в RecordSortedList (правда хрен его знает - сбрасывает он данные на диск или нет. Если данных не много - поиск по нему идет примерно с той же скоростью что и по мапам)

Ну про RecordSortedList я тоже думал, но так проще
Вообще может быть еще можно построить кучу индексов по временной таблицe, оптмизированых под именно этот запрос

[Logger]

Поковырял еще тему с оптимизацией
см. еще
axdaily: SQL temporary tables in AX 2012

Оказалось, что нелинейность прячется не в классе Tax а при прописывании налогов в строки документа.

Например для класса PurchFormLetter_Invoice в методе writeTaxAmount_W
есть код

invoiceTrans.initFromTaxWorkTrans_RU(this.tmpTaxWorkTrans(vendInvoiceJour.RecId),
                                              tablenum(PurchLine),
                                              0,
                                              invoiceTrans.InventTransId);

А метод tmpTaxWorkTrans зачем то дублирует временную табличку.
Получается что мы перебираем N строк накладной и на каждой строке копируем времянку через while select { buffer.insert() } т.е. тоже выполняем N шагов. Итого сложность алгоритма N^2 - квадратичная.

Исправить можно например так :

protected void writeTaxAmount_W()
{
    VendInvoiceTrans invoiceTrans;
    CRSEFeatures_W   features = CompanyInfo::features_W();
    TmpTaxWorkTrans     GRD_locTmpTaxWorkTrans; //+GRD_R4719_SpeedUpFormLetter_pkoz, pkoz, 24.11.2011

    if (features != CRSEFeatures_W::PL &&
        features != CRSEFeatures_W::RU)
        return;

    if (! TaxParameters::find().TaxSpecifyLine)
        return;

    // GRD_R4719_SpeedUpFormLetter_pkoz, pkoz, 24.11.2011 -->
     GRD_locTmpTaxWorkTrans = this.tmpTaxWorkTrans(vendInvoiceJour.RecId);
    // GRD_R4719_SpeedUpFormLetter_pkoz, pkoz, 24.11.2011 <--

    while select forupdate invoiceTrans
        index hint InvoiceIdx
        where invoiceTrans.PurchID             == vendInvoiceJour.PurchId           &&
              invoiceTrans.InvoiceId           == vendInvoiceJour.InvoiceId         &&
              invoiceTrans.InvoiceDate         == vendInvoiceJour.InvoiceDate       &&
              invoiceTrans.InternalInvoiceId   == vendInvoiceJour.InternalInvoiceId &&
              invoiceTrans.NumberSequenceGroup == vendInvoiceJour.NumberSequenceGroup
    {        
        invoiceTrans.initFromTaxWorkTrans_RU(
// GRD_R4719_SpeedUpFormLetter_pkoz, pkoz, 24.11.2011 -->
                                             //this.tmpTaxWorkTrans(vendInvoiceJour.RecId),
                                             GRD_locTmpTaxWorkTrans,
// GRD_R4719_SpeedUpFormLetter_pkoz, pkoz, 24.11.2011 <--
                                             tablenum(PurchLine),
                                             0,
                                             invoiceTrans.InventTransId);
        invoiceTrans.doUpdate();
    }
}

Для других классов семейства FormLetter исправляется аналогично.
На документах из 1000 строк экономия времени составила примерно 10 минут. Для документов из 100 строк не замерял, но думаю что немного.

Интересно что эта ошибка тянется еще с Ax 3.0

[Logger]

Кстати, иногда можно слышать мнение, что кешированием в памяти чего-либо можно решить большинство проблем производительности.
К сожалению, это не всегда так. Серебряной пули нет. Кривой алгоритм может поставить на колени даже память с мощным процессором.

Второй пример нелинейности при обработке накладной по закупке :
Стек вызовов :
...
PurchFormLetter.run
PurchFormLetter.createJournal
PurchFormLetter.insertJournal
PurchFormLetter_Invoice.updateNow
PurchFormLetter_Invoice.updateInventory
MarkupAdjustment::adjustInvoice
ledgerVoucherObject.postCurrencyDiff

в методе ledgerVoucherObject.postCurrencyDiff есть вызовы методов
ledgerVoucherObject.listCurrencyAmountCur
ledgerBondClient.log2Table
ledgerBondClient.currentLog

Время выполнения каждого метода также квадратично зависит от числа строк в накладной. Но тут все немного хуже чем в первом случае, потому что для накладной из 1000 строк у меня получились дублирования мапов с одновременной фильтрацией путем перебора 8-9 тысяч элементов. Этого уже не выдерживает даже супербыстрый проц и память. Самое интересно что в мооем примере не было никаких накладных расходов и судя по всему выполнение метода MarkupAdjustment::adjustInvoice можно было вообще исключить.

Пока добавил такую оптимизацию :

/// <summary>
///    Calculates MST difference per transaction and generates an equalizing transaction.
/// </summary>
/// <param name="_ledgerVoucher">
///    The ledger voucher for the transaction.
/// </param>
/// <param name="_ledgerPostingType">
///    The posting type for the transaction.
/// </param>
/// <param name="_ledgerAccount">
///    The ledger account for the transaction.
/// </param>
/// <param name="_dimension">
///    A dimension.
/// </param>
/// <param name="_sourceTableId">
///    The table ID of the posting source.
/// </param>
/// <param name="_sourceRecId">
///    The record ID of the posting source
/// </param>
/// <param name="_transactionTxt">
///    A transaction text.
/// </param>
/// <param name="_level">
///    The level; option.
/// </param>
public void postCurrencyDiff(
    LedgerVoucher       _ledgerVoucher,
    LedgerPostingType  _ledgerPostingType,
    LedgerAccount      _ledgerAccount,
    Dimension          _dimension,
    tableId            _sourceTableId,
    recId              _sourceRecId,
    TransactionTxt     _transactionTxt,
    Integer            _level = 0)
{
    LedgerVoucherTransObject    ledgerVoucherTransObject;
    RecordSortedList            rsL;
    TmpLedgerTrans              tmpLedgerTrans;
    boolean                     more;
    // <GEEU>
    TmpLedgerBondLogTable_RU    logTable;
    LedgerBondTransObject_RU    bondTransObject;
    // </GEEU>
    boolean                     GRD_LoadedBondInfo = false;//+GRD_R4719_SpeedUpFormLetter_pkoz, pkoz, 24.11.2011


    rsL = this.listCurrencyAmountCur(_level);

    // <GEEU>

    // GRD_R4719_SpeedUpFormLetter_pkoz, pkoz, 24.11.2011 -->
    if (GRD5::isR4719_2())
    {
        // вынесли этот код (блок else) внутрь цикла ниже, так как вызовы ledgerBondClient.currentLog() и ledgerBondClient.log2Table(
        // могут быть сложны для расчета при большом числе строк в документе (нелинейные зависимости от числа строк содержатся )
        // а их результат иногда бывает и не нужен, поэтому раньше времени их не вызваем, а только по мере необходимости, т.е. на первом шаге цикла
    }
    else
    {
        if (ledgerBondClient)
        {
            logTable = ledgerBondClient.log2Table(ledgerBondClient.currentLog());
        }
    }
    // GRD_R4719_SpeedUpFormLetter_pkoz, pkoz, 24.11.2011 <--

    // </GEEU>
    for (more = rsL.first(tmpLedgerTrans);
         more;
         more = rsL.next(tmpLedgerTrans))
    {
        ledgerVoucherTransObject = LedgerVoucherTransObject::newCreateTrans(
                                           this,
                                           _ledgerPostingType,
                                           _ledgerAccount,
                                           _dimension,
                                           tmpLedgerTrans.CurrencyCode,
                                           -tmpLedgerTrans.AmountCur,
                                           _sourceTableId,
                                           _sourceRecId);

        ledgerVoucherTransObject.parmTransTxt(_transactionTxt.txt());
        _ledgerVoucher.addTrans(ledgerVoucherTransObject);
        // <GEEU>

        if (ledgerBondClient && tmpLedgerTrans.AmountCur)
        {

            // GRD_R4719_SpeedUpFormLetter_pkoz, pkoz, 24.11.2011 -->
            if (GRD5::isR4719_2())
            {
                if (!GRD_LoadedBondInfo)
                {
                    GRD_LoadedBondInfo = true;
                    if (ledgerBondClient)
                    {
                        logTable = ledgerBondClient.log2Table(ledgerBondClient.currentLog());
                    }
                }
            }
            // GRD_R4719_SpeedUpFormLetter_pkoz, pkoz, 24.11.2011 <--

            bondTransObject = ledgerBondClient.bondTransObject(ledgerBondClient.lastVrefId());
            while select logTable
                where logTable.CurrencyCode == tmpLedgerTrans.CurrencyCode   &&
                      logTable.Crediting    != bondTransObject.remainCrediting()
            {
                ledgerBondClient.bondVRef2VRef(bondTransObject.vrefId(), logTable.vRef);
                if (bondTransObject.remainAmountCur() == 0)
                {
                    break;
                    // </GEEU>
                }
            }
        // <GEEU>
        }
    }
}
// </GEEU>

т.е. если для конкретных строк нет проводок, то часть копирований не происходит, за счет чего тоже экономится прилично времени.

P.S.
Вообще, грустно, что обработка накладных совсем не оптимизировалась на обработку больших объемов информации.

[Logger]

GRD5::isR4719_2() - это признак "включенности" изменений. Должен возвращать true

[someOne]

Цитата:

Сообщение от Logger

Поковырял еще тему с оптимизацией
...
Например для класса PurchFormLetter_Invoice в методе writeTaxAmount_W

Да, существенная оптимизация, спасибо за приведенный способ !!!

Цитата:

Сообщение от Logger

Для других классов семейства FormLetter исправляется аналогично.

SalesFormLetter_Invoice все то же самое. Ужас какой то...

Надеюсь "Локализаторы" обратят внимание и исправят в новых версиях!

Для тех кто работает с большим количеством строк в заказах или закупках эта оптимизация будет очень полезна.

[Wamr]

немного истории

[mazzy]

вы не горячитесь оптимизировать.

tmptaxWorkTrans содержит не только рассчитанные системой данные, но и введенные пользователем суммы с коррекциями (коррекции копеек).

к сожалению, этот момент очень запутан и плохо продуман в Аксапте.
Изменяя функционал tmptaxWorkTrans можно загубить возможность коррекции копеек в налогах.

[Logger]

Цитата:

Сообщение от mazzy

вы не горячитесь оптимизировать.
...
Изменяя функционал tmptaxWorkTrans можно загубить возможность коррекции копеек в налогах.

Спасибо за предупреждение.
Вроде бы не должно сломаться. Я проверил тот код, который изменял. Работает идентично. Я ведь не меняю сам расчет. Указанный код работает в самом конце обработки, после расчета всех налогов и учета всех коррекций - он просто прописывает суммы в строки документов. Я поправил не расчет, а именно перенос итоговых сумм налогов в строки.

[Logger]

Походил по ссылкам, наткнулся на
http://forum.mazzy.ru/index.php?showtopic=1790

Розничные продажи - как раз получают описанные проблемы, потому что там строк в документах дофига (счет идет на тысячи), и проблема не только в индексах и БД, а в кривом коде, который зря грузит сервер приложений.

[someOne]

Цитата:

Сообщение от Wamr

немного истории

Мммда... С 2005 года - могли бы десять раз поправить уже. Мне казалось что люди из команды разработчиков Аксапта читают этот форум... Жаль. Может теперь заметят ?

[Logger]

Цитата:

Сообщение от Wamr

немного истории

Гыгы.
Я изобрел велосипед

А вы, случаем, свою находку тогда в 2005-м не регали в MS ?