Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Рефакторинг, оптимизация и переписывание нового кода #310

Closed
Mazdaywik opened this issue Jul 4, 2020 · 1 comment · Fixed by #328
Assignees
Labels

Comments

@Mazdaywik
Copy link
Member

В рамках задач #252 (@Kaelena), #253 (@koshelevandrey), #260 (@Cstyler), #256 и #273 (@nexterot) в компилятор было внесено много нового кода, в общей сложности около 6000 строк. Бо́льшую часть этого кода я внимательно не изучал. Поверхностный взгляд выявил в нём неоптимальности и отступления от принятого стиля кодирования.

Поэтому в рамках данной задачи нужно будет проинспектировать новый код и выполнить рефакторинг, оптимизацию, и, если надо, переписывание:

  • Рефакторинг будет сводиться к стилевым правкам, прояснениям имён функций и переменных, использованием более идиоматичного кода вместо менее идиоматичного. Он не меняет поведение программы и сводится лишь к улучшению её структуры. Кроме того, в процессе работы местами был выявлен старый кривой код, рефакторинг которого мы отложили на потом (см. Интринсики для встроенных функций #260 (comment), около смайлика «🤦‍♂️»).
  • Оптимизация будет сводиться к поиску узких мест в новом (а может быть, и в старом) коде и их ускорению. В принципе, оптимизация уже была выполнена там, где это было критично для прохождения тестов (Проблемы, замеченные на случайных тестах #305). Оптимизация должна, как и рефакторинг, сохранять поведение компилятора.
  • Переписывание кода будет выполняться там, где ускорить его посредством оптимизации нельзя, а также нельзя улучшить структуру рефакторингом, то есть с сохранением имеющегося поведения. Жалко будет выкидывать имеющиеся наработки, но придётся.
@Mazdaywik Mazdaywik added the task label Jul 4, 2020
@Mazdaywik Mazdaywik self-assigned this Jul 4, 2020
Mazdaywik added a commit that referenced this issue Jul 4, 2020
Осторожные стилевые правки:
• оформление кода — отступы, переносы на другую строку,
• внутри Map: e.Other → t.Other,
• функция с двумя форматами разбита на две.
Mazdaywik added a commit that referenced this issue Jul 4, 2020
При компиляции с -Wall получаем следующее сообщение:

OptTree-AutoMarkup-SpecializableExtractor.ref:246:1:
  WARNING: Function `GetVariablesTypes':
  sentence $5 screens sentence $6 [-Wscreening]

(переносы строк добавлены для читабельности)

На самом деле, я проблему заметил, когда делал стилевые правки. Просто
мне хотелось запустить с -Wall и увидеть предупреждение.
Mazdaywik added a commit that referenced this issue Jul 5, 2020
Мне не хотелось переписывать код целиком, хотелось сохранить структуру
оригинальных алгоритмов. Но пришлось. Исходная реализация была громоздкой
и трудной для оптимизации. Написал более компактную, быструю и лаконичную.
Mazdaywik added a commit that referenced this issue Jul 14, 2020
• Прояснение форматов функций (в частности, удалены избыточные форматные
  скобки).
• Удаление недостижимых предложений.
• Уточнена структура алгоритма для последующих исправлений ошибок.
• Переименованы некоторые функции и поправлено форматирование кода.
Mazdaywik added a commit that referenced this issue Jul 14, 2020
Вместо шаблона специализации по ошибке строился список переменных
из обобщения, переменным давались индексы .STAT#n и .dyn#n.
Из-за того, что ранее индексы формировались неправильно (38523f9),
специализация не выполнялась — вызов считался с тривиальной сигнатурой
и ошибка не воспроизводилась.
Mazdaywik added a commit that referenced this issue Jul 14, 2020
При запуске автотестов выяснилось, что функции GetVariableMatches
и GetVariablesSpecType неправильно помечают переменные из образца
статическими.

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

Теперь всё работает.
Mazdaywik added a commit that referenced this issue Jul 18, 2020
• Уточнён тип переменной-флага e.NeedRelationCheck → s.NeedRelationCheck.
• Мелкие стилевые исправления.
Mazdaywik added a commit that referenced this issue Jul 22, 2020
• Оформление кода.
• Длинные строки.

На самом деле там нужно много чего переделывать. Здесь я поправил участок
кода, с которым нужно работать в рамках #320 и длинные строки, чтобы
не появлялась горизонтальная прокрутка.
Mazdaywik added a commit that referenced this issue Oct 23, 2020
• Переименование функции,
• уточнение типов переменных,
• прочая мелочь.
Mazdaywik added a commit that referenced this issue Oct 23, 2020
И это приводило к зацикливанию, которое демонстрирует автотест.
Mazdaywik added a commit that referenced this issue Oct 23, 2020
Написан новый алгоритм построения графа вызовов функций. В нём исправлены
следующие проблемы:

• Восстановлена вставка ссылок на функции вместо указателей на метатаблицы.
• Учитывается, что метафункции могут осуществлять косвенный вызов.
• Функции, осуществляющие косвенный вызов, вызывают все функции, на которые
  берутся указатели.

При этом для прояснения коммита весь старый код в файле присутствует. Старый
код будет удалён в следующем коммите.

Ссылка на заявку #310 означает, что немалая часть кода была переписана.
Mazdaywik added a commit that referenced this issue Oct 23, 2020
Удалён код, ставший неактуальным после предыдушего коммита.
Mazdaywik added a commit that referenced this issue Oct 23, 2020
…#310)"

This reverts commit 0a5db2a.

Коммит отменяется, т.к. функции, помеченные $INLINE, по определению
рекурсивные. Если бы было бы иначе, их можно было бы пометить $DRIVE.

А то, что пометка приводит к зацикливанию — значит, пометка неправильная.
В отличие от $DRIVE с -OA и $SPEC, пометка $INCLUDE по-прежнему опасная.

Коммит я не стал удалять из истории, а вместо этого его реверсировал
для того, чтобы оставить этот поучительный комментарий.
Mazdaywik added a commit that referenced this issue Oct 25, 2020
)

В дальнейшем в этом файле будет аккумулирована вся логика расстановки меток
$DRIVE: вставка недостающих, удаление избыточных.
Mazdaywik added a commit that referenced this issue Oct 25, 2020
Также сами перенесённые функции были слегка переписаны.
Mazdaywik added a commit that referenced this issue Nov 17, 2020
Для поиска используется хэширование, которое позволяет снизить сложность
с линейной до квадратного корня. Размер хеша (101) оптимизирован для случая
≈10 000 функций.
Mazdaywik added a commit that referenced this issue Nov 17, 2020
Тоже используется хеширование, в данном случае — хеш-множество.
Mazdaywik added a commit that referenced this issue Nov 22, 2020
Почти, т.к. порядок корней меняется, а значит, могут измениться базисные
вершины. Но на корректность это никак не влияет.

Не обязательно добавлять в список корней имена __INIT и __FINAL только
когда такие функции есть в графе. Их можно добавлять всегда, алгоритм
обхода графа их просто пропустит, не найдя в графе.
Mazdaywik added a commit that referenced this issue Nov 22, 2020
Порядок имён в списке корней может влиять на выбор базисных вершин.
Мы их помещаем в самый конец, поскольку эффективнее их пропустить на самой
последней итерации, когда граф уже почти пустой, чем на самой первой.
Mazdaywik added a commit that referenced this issue Nov 22, 2020
Было заманчиво написать

    <ApplyInlines e.Inlines (e.Inlines)>

и сделать функцию MakeInlineClosure с одним аргументом, но это оказалось
немного медленнее. Поэтому там e.InlinesAcc, который насыщается
до неподвижной точки.
Mazdaywik added a commit that referenced this issue Nov 25, 2020
Функция StrFromMacroDigit переименована в Symb-Digit по аналогии
с Add-Digits и другими. Автотест добавлен для уверенности — для покрытия
нового первого предложения функции (ранее тесты проверяли только длинные
числа).
Mazdaywik added a commit that referenced this issue Nov 26, 2020
Учтено, что на Linux может требоваться больше памяти на больший префикс,
поэтому лимит памяти для компилятора повышен.
@Mazdaywik
Copy link
Member Author

Код приемлем по быстродействию, новых критичных ошибок пока не вылезло. Правки @nexterot’а по экранированию предложений я не проверял, поскольку это пока побочная функциональность.

Mazdaywik added a commit that referenced this issue Nov 27, 2020
Почему-то (видимо, невнимательный копипаст) все функции программы помещались
в список e.Forbidden, из-за чего они не прогонялись.
Mazdaywik added a commit that referenced this issue Nov 29, 2020
Ранее это не приводило к проблемам, т.к. ни прогонка не осуществляется
в функциях с условиями, не сами функции с условиями не подлежат прогонке.
Mazdaywik added a commit that referenced this issue Nov 29, 2020
Поддержка была реализована на основе графа вызовов функций, используемом
для авторазметки (#252). Реализованный алгоритм обхода графа не только
выявлял функции, пригодные и не пригодные для прогонки, но и функции
недостижимые. Этим и воспользовались.

Граф строится по упрощённой схеме: не выполняются встраивания косвенных
вызовов, метатаблиц и inline-функций. Всё это нужно для разметки
прогоняемых вершин, но избыточно в рамках настоящей задачи.

Корнями графа при обходе считаются все функции без суффиксов — так проще
не в ущерб корректности.

При построении в граф также добавляются теги АТД-скобок как указатели.
На корректность разметки для прогонки это никак не влияет, поскольку
теги абстрактных скобок обычно являются пустыми функциями. Но они нужны
алгоритму чистки, иначе определения тегов будут удалены и программа станет
некорректной.
Mazdaywik added a commit that referenced this issue Dec 6, 2020
Во-первых, они не помечаются авторазметкой для прогонки,
т.к. не помещаются в граф.

Во-вторых, они помечаются как запрещённые для прогонки.

В-третьих, модуль прогонки не рассматривает пустые хвосты как прогоняемые.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
1 participant