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

[Vadik]

Цитата:

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

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

единственный способ это исправить - зарегистрировать соответствующий запрос в поддержке

[Zabr]

Вставлю свои 5 копеек про таблицу TaxTrans без индекса и крайне тормозной расчет "Налога 1099" : Загрузка строк Purchase order

[Logger]

Цитата:

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

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

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

Кстати такой вариант оптимизации времянок мы попробовали.
Идея в том, чтобы заставить времянки создаваться в памяти.
Например, можно завести виртуальный диск, реально живущий в памяти и прописать его в переменную окружения как темповую папку для пользователя, под которым работает сервер приложения. В таком случае файлы *.$$$, содержащие времянки будут реально жить в памяти и работать быстро. К сожалению, это не решило всех проблем. Если поиск во времянке делать по неиндексированному полю, то время улучшается примерно в 6-7 раз. Если по индексированному, то в 2 раза. (Т.е. индексы на больших времянках тоже существенно влияют на скорость) Также примерно в 2 раза ускоряется операция вставки. Мы ожидали бОльшего ускорения, поэтому на боевой базе не стали это применять.

Тест проводили на примерно 100-200 тысячах записей буферов UnitConvert. Это составило примерно 50-100 мегабайт в $$$ файле на диске.

[Masel]

Я тут в очередной раз занимался оптимизацией разноски розничных продаж и наткнулся еще на пару возможных улучшений.
1. Метод writeTaxAmount_W, который тут ранее оптимизировали вызывает в цикле метод CustVendInvoiceTrans.initFromTaxWorkTrans_RU(). Там выполняется неиндексируемый запрос к злосчастной темповухе TmpTaxWorkTrans. Единственный селективный фильтр там это номер лота InventTransId, но индекса по нему нету. Получается примерно такая трассировка

В общем нужно добавить индекс и плюс пришлось еще поменять немного код на поиске.

    if (SysCountryRegionCode::isLegalEntityInCountryRegion([#isoRU]))
    {
        //+ sergey.m 03.02.2022 FRE_20421639_001
        if (!_sourceRecId)
        {
            select taxWorkTrans index hint InventTransIdx
                where taxWorkTrans.SourceTableId == _sourceTableId       &&
                      taxWorkTrans.InventTransId == _inventTransId       &&
                      taxWorkTrans.TaxDirection  != TaxDirection::UseTax &&
                      taxWorkTrans.TaxOrigin     != TaxOrigin::TaxReversed;
        }
        else
        {
        //- sergey.m 03.02.2022 FRE_20421639_001
            select taxWorkTrans
                where taxWorkTrans.SourceTableId  == _sourceTableId             &&
                      ((_sourceRecId &&
                        taxWorkTrans.SourceRecId == _sourceRecId) ||
                       (! _sourceRecId &&
                        taxWorkTrans.InventTransId == _inventTransId))          &&
                      taxWorkTrans.TaxDirection       != TaxDirection::UseTax   &&
                      taxWorkTrans.TaxOrigin          != TaxOrigin::TaxReversed;
        }

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

2. В классе Tax метод lineTaxAmount. В начале метода проверяется что в таблице есть записи немного экзотическим методом, считая их количество. Я поменял так, хотя можно было наверное вообще убрать запрос, меня пока и так устраивает.

    if (this.taxParameters().TaxSpecifyLine)
    {
    if (this.taxParameters().TaxSpecifyLine)
    {
        //+ sergey.m 03.02.2022 FRE_20421639_001
        //select count(RecId) from taxWorkTrans;
        select firstOnly RecId from taxWorkTrans;
        //- sergey.m 03.02.2022 FRE_20421639_001

        if (taxWorkTrans.RecId > 0 && !this.useSubLedgerJournalLines())
        {
            // Posting out of TmpTaxWorkTrans

Еще тормозит российская корреспонденция фин. проводок. Так не хочется глубоко погружаться в это. Никто не правил случаем? Я там кое-что правил, но вот этот кошмар остался. Без единового вызова субд.

[Logger]

Цитата:

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

Еще тормозит российская корреспонденция фин. проводок. Так не хочется глубоко погружаться в это. Никто не правил случаем? Я там кое-что правил, но вот этот кошмар остался. Без единового вызова субд.

Для какой версии ?
Я для 2009-й кое что правил - некие частные случаи с накладными расходами.
Там проблема из-за нелинейной зависимости от числа строк при разноске. (где то на форуме была тема)

Где возможно -
1. отключали корреспонденцию.
2. для больших документов (более 300 строк) вместо одной накладной делали кучку накладных по 40 строк. тем самым линеаризовали зависимость времени разноски от числа строк. - у нас были компании для управленческого учета где число накладных было неважно, главное итоговые показатели. Вот там такое было применимо.

[Masel]

Цитата:

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

Для какой версии ?

2012 R2. Подозреваю что существенно не поменялось с 2009. В CIL то оно еще сносно работает, но все равно бы хотелось оптимизировать. Там явно неэффективные эти пробеги по map бесконечные.

[Logger]

а, так вот пример
Оптимизация класса Tax

ну можно что-то аналогичное сделать.

Вообще код сложный. Стремно что-то менять.
Может разноску в ГК разбить на куски - сгруппировать по N строк. Уйдут нелинейные эффекты.
Я думаю так в итоге проще будет и безопаснее. Меньше вероятности что-то сломать. Обновления проще поднять.

Про нелинейность:
Как линейное время превращается в Windows в O(n²)

[Logger]

Цитата:

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

В CIL то оно еще сносно работает, но все равно бы хотелось оптимизировать. Там явно неэффективные эти пробеги по map бесконечные.

В CIL быстрее за счет двух вещей
1. JIT (ну и в целом CIL шустрее чем p-code)
2. Асинхронный сборщик мусора.

[Logger]

Цитата:

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

Там явно неэффективные эти пробеги по map бесконечные.

Да!
Да!

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

[EVGL]

Бессмысленное занятие. В D365 все уходит в microservice с закрытым кодом, а для индийского GST они в Москве все давно перекроили на решение систем линейных уравнений, кажется.

https://docs.microsoft.com/en-us/dyn...ion-service-ga

П.с. говорит автор. Моя работа
Думаю, вы правы. На этапе merge я, кажется, проверял соответствие ряда характеристик LedgerVoucherTransObject, как то: счет ГК, аналитики, тип проводки, сторона дебет/кредит. Наверное, из них можно было собрать hash key. Тогда (2002) такого pattern не было.

[Logger]

Цитата:

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

Бессмысленное занятие.

Бессмысленное, если в ближайшем будущем планировать переход на 365ю.

[SRF]

Цитата:

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

Я тут в очередной раз занимался оптимизацией разноски розничных продаж и наткнулся еще на пару возможных улучшений.
1. Метод writeTaxAmount_W, который тут ранее оптимизировали вызывает в цикле метод CustVendInvoiceTrans.initFromTaxWorkTrans_RU(). Там выполняется неиндексируемый запрос к злосчастной темповухе TmpTaxWorkTrans. Единственный селективный фильтр там это номер лота InventTransId, но индекса по нему нету. Получается примерно такая трассировка

На уровне идеи (не факт конечно, что будет быстрее) - можно же вообще убрать запрос в этом месте к темповой табличке для RU функциональности(наверное и в общем случае можно, но будет чуть сложнее), я может конечно что то забываю или не учитываю, но идея такая :

- на одну строку накладной одна запись в TmpTaxWorkTrans для RU, связь по номеру лота (по логике можно хранить и несколько записей, но кажется в этом случае особого профита по сравнению с индексом не будет)

- получается можно до цикла по строкам получить map - ключ inventTransId, значение запись TmpTaxWorkTrans(в методе который собственно и формирует всю TmpTaxWorkTrans, т.е. дополнительных проходов не потребуется).

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

[Masel]

Цитата:

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

На уровне идеи (не факт конечно, что будет быстрее) - можно же вообще убрать запрос в этом месте к темповой табличке для RU функциональности(наверное и в общем случае можно, но будет чуть сложнее), я может конечно что то забываю или не учитываю, но идея такая :

- на одну строку накладной одна запись в TmpTaxWorkTrans для RU, связь по номеру лота (по логике можно хранить и несколько записей, но кажется в этом случае особого профита по сравнению с индексом не будет)

- получается можно до цикла по строкам получить map - ключ inventTransId, значение запись TmpTaxWorkTrans(в методе который собственно и формирует всю TmpTaxWorkTrans, т.е. дополнительных проходов не потребуется).

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

Не факт что там одна запись, индексов уникальных нет и выбирется курсор без firstonly, внутри они его фетчат, делают next TmpTaxWorkTrans. Поэтому придется делать map InventTransId -> Set(Types::record) и потом внутри логику менять, чтобы он заместо фетча бегал по Set. Это можно сделать, но в моем сценарии нет смысла, т.к. индексированный поиск отрабатывает меньше милисекунды.

[Masel]

Оптимизировал все таки корреспонденцию. Тестовый пример заказ на 2770 строк. В CIL выигрыш в абсолютном выражении не большой: время сократилось с 1,5 минут до 18 секунд. А в p-code c 20 минут до 1 мин 35сек. Медленно работало объединение списков ссылок на проводки TaxTrans при суммировании проводок ГК в корреспонденции. Это делалось поэлементно, переделал на контейнеры.

[Logger]

Цитата:

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

Оптимизировал все таки корреспонденцию...

А можете выложить некастомизированную, стандартную версию класса для вашей версии приложения ?
Думаю это поможет тем кто захочет портировать на 2009-ю версию.

Плюс с R3 тоже есть незначительные отличия.

[Masel]

Исходная с R2 во вложении.

[mazzy]

Цитата:

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

Оптимизировал все таки корреспонденцию. Тестовый пример заказ на 2770 строк. В CIL выигрыш в абсолютном выражении не большой: время сократилось с 1,5 минут до 18 секунд. А в p-code c 20 минут до 1 мин 35сек. Медленно работало объединение списков ссылок на проводки TaxTrans при суммировании проводок ГК в корреспонденции. Это делалось поэлементно, переделал на контейнеры.

1.
Спасибо!

2.
Для длинных коллекций в Аксапте вместо контейнеров лучше использовать List.
Контейнер только в 2012 и только в CIL реализован на базе System.Array
в младших версиях и в p-code контейнер реализован на базе строки.

3.
смотрите что сейчас у вас получается.
3.1. вы получаете коллекцию taxTransactionRelationshipCollection
3.2. эта коллекция по сути дела является Map(Key:RecId, Value:Set{Enum})
3.3. вы перекладываете этот map в 4 ваших контейнера (кстати, enum'ов больше, чем 4)
3.4. а в методе taxTransactionRelationshipCollection() вы перекладываете ваши контейнеры в новую коллекцию taxTransactionRelationshipCollection.

вопрос: а зачем вообще нужна эта перекладка из мапы в контейнер и наоборот?
добавьте метод merge в класс LedgerPostingOneToManyCollection и пользуйтесь доступом непосредственно к методам мапы и множества - insert, lookup, in, add. Получится и проще для понимания и быстрее.

вопрос: а зачем вообще нужна мапа, состоящая из множеств? для которой нужно реализовывать операции с каждым элементом.

Если уж решились оптимизировать - используйте временную таблицу.
тогда сложение двух коллекций будет реализовано оператором insert_recordset.

Причем используйте временную таблицу прямо внутри класса LedgerPostingOneToManyCollection.
Оставьте внешний интерфейс класса неизменным, а внутреннюю реализацию сделайте на временной таблице.

------------------------------------

Скорее всего, классы LedgerPostingOneToManyCollection и LedgerBondMergeablesGeneralJour_RU реализовывали разные люди.
И русские скорее всего не могли вставиться в LedgerPostingOneToManyCollection по огранизационным причинам.
Но вы то можете.

Обратите внимание насколько код в 2012 НЕ использует возможности таблиц и возможности СУБД. Бегут как чёрт от ладана. Всякую херню в памяти делают, лишь бы не таблицы.
Но вы то не такие. Правда?

------------------------------------

и... фу, какие душные названия в ax2012.

[Masel]

Цитата:

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

Для длинных коллекций в Аксапте вместо контейнеров лучше использовать List.
Контейнер только в 2012 и только в CIL реализован на базе System.Array
в младших версиях и в p-code контейнер реализован на базе строки.

List здесь не подходит, потому что его нельзя быстро клонировать. pack/unpack вызывает ту же многократную вставку. Я это проверял.

Цитата:

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

3.3. вы перекладываете этот map в 4 ваших контейнера (кстати, enum'ов больше, чем 4)

На R2 в енуме 4 значения. У вас их больше либо потому, что R3, либо потому, что добавлено на USR слое.


Вы в принципе не совсем понимаете в чем затыки по производительности в этом месте (может в ваших сценариях их просто нет). У нас есть большое количество кусочков проводок ГК по налогам, образованные каждой строкой заказа на продажу. Дальше упрощая система бежит по этим кусочкам и прибавляет их к первой проводке, получается как бы нарастающий итог. В моем случае все эти проводки сворачиваются в итоге всего в 2 проводки по налогам, вроде на продажи и возвраты (точно не разбирался, вторая с минусом). Кроме суммирования самих полей проводки еще объединяются ссылки на taxTrans, это recid. Поэтому типичная ситуация, когда к проводке, у которой уже 7000 ссылок прибавляется одна по которой 1 ссылка. Система это делает следующим образом:

    #define.IgnoreDuplicates(true)
    LedgerPostingOneToManyCollection ret = LedgerPostingOneToManyCollection::construct(
        Types::Int64, // use this instead of RecId for perf
        Types::Enum,
        #IgnoreDuplicates);

    this.addCollections(ret, _first);
    this.addCollections(ret, _second);

То есть она создает абсолютно новую структуру данных и переваливает потом все элементы в цикле по одному. В пером параметре _first куча элементов, во втором немного. Здесь идеально бы работало просто

this.addCollections(_first, _second);
return _first;

Но тогда вы измените taxTransactionRelationshipCollection на которую ссылаются из других мест. Там есть целый класс, который еще раз ссылается на все эти свойства и хранит их ID. В целом по коду кажется что после суммирования старый экземпляр LedgerBondMergeablesGeneralJour_RU уже использоваться не будет, но я так делать не решился.

Далее встает вопрос, как переделать так, чтобы оставить такой подход - два элемента скопировать в первый, но сделать это быстро. Нужно что-то типа _first.Clone(). Но такого метода ни у Map, ни у Set нету. pack/unpack не решают задачу.

В качестве решения предлагается изменить структуру хранения так, чтобы дублирование выполнялось командой container1 = container2, это работает быстро. А для того, чтобы изменить интерфейс работы с классом модифицировать taxTransactionRelationshipCollection метод, чтобы он выдавал данные в привичном виде, собирая из контейнеров результат.

Сценарий работы с этим классом такой.
1) Вначале экземпляры создаются по каждой микропроводке по строке заказа методом initFromLedgerVoucherTransObject
2) Все многократно мерджится, используется метод merge, вызывается много раз
3) По просуммированным проводкам вызывается 1 раз метод taxTransactionRelationshipCollection по каждой проводке. У меня проводки 2, соответственно вызывается он по налогам 2 раза.

Поэтому оптимизируем вызовы merge.

По поводу идеи с таблицей. Будет тормозить еще больше я думаю, хотя не пробовал. В CIL точно больше.
Вместо 4 контейнеров можно было сделать один, который бы хранил и RecId и enum. Но он был бы в 2 раза больше. У меня в примере например значение енума Offset всегда, соответственно только 1 полный контейнер.
Можно еще как-то по другому делать, например я думал использовать структуры .Net. Или, возможно если оставить эти пробеги в циклах по Map, но просто убрать Set, то этого будет достаточно (заменить Map recId -> Set, на более простую структуру данных). Но меня просто устраивает этот вариант, я его замерил, если знаете как лучше, то сделайте, только проверьте результат по времени работы.

[mazzy]

Цитата:

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

На R2 в енуме 4 значения. У вас их больше либо потому, что R3, либо потому, что добавлено на USR слое.

R3

Цитата:

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

Вы в принципе не совсем понимаете в чем затыки по производительности в этом месте

Цитата:

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

У нас есть большое количество кусочков проводок ГК по налогам, образованные каждой строкой заказа на продажу.

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

Цитата:

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

То есть она создает абсолютно новую структуру данных и переваливает потом все элементы в цикле по одному. В пером параметре _first куча элементов, во втором немного.

Угу.

Цитата:

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

Здесь идеально бы работало просто

this.addCollections(_first, _second);
return _first;

Но тогда вы измените taxTransactionRelationshipCollection на которую ссылаются из других мест.

Угу-угу.

Цитата:

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

В целом по коду кажется что после суммирования старый экземпляр LedgerBondMergeablesGeneralJour_RU уже использоваться не будет, но я так делать не решился.

угу-угу-угу.
авторы россикского класса вот тоже побоялись.

Цитата:

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

Далее встает вопрос, как переделать так, чтобы оставить такой подход - два элемента скопировать в первый, но сделать это быстро.

Нужно что-то типа _first.Clone(). Но такого метода ни у Map, ни у Set нету. pack/unpack не решают задачу.

tmpTable.setTmpData(fromTmpTable);



по поводу pack/unpack создал отдельную тему
List/Set/Map/Array: deep clone через pack/create в классических Аксаптах

Цитата:

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

В качестве решения предлагается изменить структуру хранения так, чтобы дублирование выполнялось командой container1 = container2, это работает быстро. А для того, чтобы изменить интерфейс работы с классом модифицировать taxTransactionRelationshipCollection метод, чтобы он выдавал данные в привичном виде, собирая из контейнеров результат.

да, это я понял.
и огромное вам спаибо за предложенное решение.

но решить можно еще лучше.

прежде всего, потому что у вас нет организационных ограничений. В Майкрософте каждый объект АОТ приписан к определенной команде. И разработчик другой команды не имеет права менять чужой объект без согласования. Поэтому разработчики Московской команды часто вынуждены делать странные решения сбоку, вместо того, чтобы просто расширить функционал другого объекта АОТ.

У вас таких ограничений нет - вы можете менять любой класс, любую таблицу, любой объект АОТ.
Очень многие странности в Аксапте можно решить просто перенеся функционал в другой класс.
Вот такая вот организация труда у них там в Майкрософте.

Подумайте - а что если бы метод merge существовал прямо в классе с идиотским назвнием LedgerPostingOneToManyCollection?

Цитата:

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

По поводу идеи с таблицей. Будет тормозить еще больше я думаю, хотя не пробовал. В CIL точно больше.

Подумайте
Подумайте не только об этом месте.

Ведь по сути, данное место просто окрашивает движения разными enum-признаками. Рано или поздно эти признаки надо будет как-то связать с исходными движениями.

Поскольку сейчас признаки хранятся в коллекции, то надо будет снова делать цикл по одному и искать исходные движения по одному.
А можно было бы сделать select-join со временной таблицей

Похоже, что беда в том, что данный код класса с идиотским названием делал человек, который не знает ни аксапту, ни СУБД. Причем, этот код прошел ревью. Т.е. либо это был team lead, который может впендюрить код без ревью, либо вся команда была уровня "не знаю ни аксапту, ни СУБД". И я даже не знаю какой вариант хуже...

Цитата:

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

Вместо 4 контейнеров можно было сделать один, который бы хранил и RecId и enum.

а можно map как в исходном классе с идиотским названием LedgerPostingOneToManyCollection

Цитата:

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

Можно еще как-то по другому делать, например я думал использовать структуры .Net.

это только для CIL.
между p-code и .net слишком дорогой маршаллинг.

Цитата:

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

Или, возможно если оставить эти пробеги в циклах по Map, но просто убрать Set, то этого будет достаточно (заменить Map recId -> Set, на более простую структуру данных).

угу. таблицу, например

Цитата:

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

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

Если устраивает, то нет вопросов, конечно.

Огромное спасибо за предложенное решение.

[Masel]

Цитата:

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

tmpTable.setTmpData(fromTmpTable);

Это не дублирование, вы просто получите вторую ссылку на тот же экземпляр таблицы.

Цитата:

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

Подумайте - а что если бы метод merge существовал прямо в классе с идиотским назвнием LedgerPostingOneToManyCollection

Я если честно не понимаю просто эту идею, что это даст.