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

[Vadik]

Цитата:

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

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

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

[Zabr]

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

[Logger]

Цитата:

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

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

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

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

[mazzy]

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

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

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

[Logger]

Цитата:

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

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

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

[Logger]

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

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

[Logger]

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

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

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

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

[EVGL]

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

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

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

[Logger]

Цитата:

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

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

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

[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

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

[Владимир Максимов]

Common.setTmpData() создает новый экземпляр при передаче между клиентом и сервером. А если на одной стороне, то проще вставку сделать. Таблица же...

    TmpFrmVirtual tmp1, tmp2;
    
    tmp1.clear();
    tmp1.ItemId = 'item1';
    tmp1.insert();
    
    
    insert_recordset tmp2 (ItemId) 
        select ItemId from tmp1;

    tmp2.clear();
    tmp2.ItemId = 'item2';
    tmp2.insert();    


    select count(RecId) from tmp1;
    info(strFmt("tmp1 = %1", tmp1.recId));    

    select count(RecId) from tmp2;
    info(strFmt("tmp2 = %1", tmp2.recId));