Обновление Discourse до Zeitwerk

Разработка
1

Rails 6 поставляется с двумя режимами автозагрузки: zeitwerk и classic. В том пул-реквесте https://github.com/discourse/discourse/pull/8083 я обновил Rails до версии 6.0.0, используя классический автозагрузчик как переходный этап. Было бы интересно попробовать переключиться на Zeitwerk.

Zeitwerk — это эффективный и потокобезопасный загрузчик кода для Ruby. Пока проект следует соглашениям об именовании, Zeitwerk может находить правильные файлы и загружать их по требованию или заранее, без необходимости использовать require или require_dependency. Кроме того, согласно этой статье https://weblog.rubyonrails.org/2019/2/22/zeitwerk-integration-in-rails-6-beta-2/, это может дать небольшой прирост производительности приложения.

Мне нужно выполнить несколько шагов, чтобы это заработало:

  1. Переименовать несколько классов, чтобы они соответствовали соглашениям об именовании в Rails. Например, файл canonical_url.rb должен определять класс CanonicalUrl, а не CanonicalURL. Аналогично, файл ondiff.rb должен определять класс Onpdiff, а не ONPDiff. Альтернативный подход — подключить собственный инфлектор к проекту, однако я считаю, что следование соглашениям может быть лучшим выбором — GitHub - fxn/zeitwerk: Efficient and thread-safe code loader for Ruby · GitHub

  2. Как и в предыдущем пункте, по соглашению пользовательские валидации, находящиеся в директории validations, должны быть обернуты в модуль Validations. Кроме того, некоторые валидации наследуются от EachValidator и должны быть доступны без пространства имен. Я планирую переместить их в отдельную директорию и добавить в пути автозагрузки.

  3. Удалить все require_dependency и убедиться, что проект работает.

  4. Удалить все require и убедиться, что Discourse работает.

  5. Убедиться, что все плагины могут получить доступ к необходимым зависимостям. Пока я не знаю, как этого добиться. Сначала я хочу запустить Discourse без каких-либо плагинов.

Конечно, всё ещё есть множество неизвестных, которые нужно решить. Я буду держать вас в курсе прогресса, однако, если вы заинтересованы в просмотре, я начал экспериментировать с этим здесь: Commits · KrisKotlarek/discourse · GitHub

Пожалуйста, дайте знать, если вы видите какие-либо недостатки внедрения Zeitwerk или считаете, что я что-то упустил.

2

Я добился некоторого прогресса в работе с Zeitwerk, однако изменил свой подход. Первоначально я планировал исправить каждое место в Discourse, где не соблюдается соглашение о именах файлов Zeitwerk. После нескольких исправлений я понял, что это лишь верхушка айсберга, и заметил, что если я пойду по этому пути, то pull request будет трудно читать и уверенно мержить в master. Например, все классы задач в обычном каталоге должны находиться в пространстве имён Regular, то же самое касается Onceoff и Scheduled.

Я решил немного отступить и подумать о более эволюционном, а не революционном подходе.

Я решил, что лучше внедрить собственный Inflector, который покроет все файлы, не соответствующие соглашению Zeitwerk. Главное преимущество в том, что мы сможем развернуть это небольшое изменение, и, как только мы будем довольны работой Zeitwerk и не увидим снижения производительности, сможем постепенно исправлять соглашение файл за файлом в рамках небольших pull request.

Я обнаружил некоторые проблемы, которые нельзя было решить с помощью кастомного Inflector, поэтому внес дополнительные исправления, чтобы всё заработало.

Pull request всё ещё в процессе, однако на данном этапе я могу запустить Discourse с Zeitwerk и стандартными плагинами, выполнить все тесты и бенчмарки без проблем.

Сначала я хотел достичь стабильного состояния, когда все тесты проходят. Теперь я могу уверенно удалять все require_dependency по одному, а также тестировать официальные плагины. Как только всё будет готово, я поделюсь с вами результатами бенчмарков в этом посте.

На данный момент, если вас интересует прогресс, вы можете посмотреть черновик PR — DEV: Upgrading Discourse to Zeitwerk by KrisKotlarek · Pull Request #8098 · discourse/discourse · GitHub

Самый важный файл — это кастомный Inflector для Zeitwerk — DEV: Upgrading Discourse to Zeitwerk by KrisKotlarek · Pull Request #8098 · discourse/discourse · GitHub

3

Чтобы плагины заработали, мне пришлось создать ещё несколько небольших pull request’ов. После их мержа, думаю, тесты в Discourse должны пройти.

Также я проверил производительность на Rails 6.0.0 с классическим автозагрузчиком и на Rails 6.0.0 с Zeitwerk.

Тест Классический Zeitwerk Процент
categories-50 32 26 81.25
categories-75 37 29 78.38
categories-90 47 35 74.47
categories-99 67 49 73.13
home-50 30 29 96.67
home-75 37 31 83.78
home-90 44 40 90.91
home-99 67 52 77.61
topic-50 35 35 100.00
topic-75 36 36 100.00
topic-90 48 36 75.00
topic-99 57 58 101.75
categories_admin-50 51 48 94.12
categories_admin-75 62 50 80.65
categories_admin-90 89 66 74.16
categories_admin-99 135 101 74.81
home_admin-50 48 47 97.92
home_admin-75 58 49 84.48
home_admin-90 67 64 95.52
home_admin-99 101 81 80.20
topic_admin-50 48 48 100.00
topic_admin-75 55 49 89.09
topic_admin-90 63 65 103.17
topic_admin-99 92 69 75.00
load_rails 2617 2165 82.73
rss_kb 282428 315684 111.78
pss_kb 270491 303504 112.20

Результаты не всегда стабильны, поэтому относитесь к ним с долей скепсиса.

4

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

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

5

Я попробовал ещё раз, вот результаты:

Тест Classic Zeitwerk Процент
categories-50 25 25 100.00
categories-75 26 26 100.00
categories-90 37 33 89.19
categories-99 57 48 84.21
home-50 26 26 100.00
home-75 27 28 103.70
home-90 38 35 92.11
home-99 60 50 83.33
topic-50 27 26 96.30
topic-75 35 27 77.14
topic-90 41 33 80.49
topic-99 54 50 92.59
categories_admin-50 48 50 104.17
categories_admin-75 60 61 101.67
categories_admin-90 76 71 93.42
categories_admin-99 122 122 100.00
home_admin-50 47 46 97.87
home_admin-75 58 55 94.83
home_admin-90 66 63 95.45
home_admin-99 99 121 122.22
topic_admin-50 50 49 98.00
topic_admin-75 62 50 80.65
topic_admin-90 72 65 90.28
topic_admin-99 103 74 71.84
load_rails 2675 2216 82.84
rss_kb 279924 315240 112.62
pss_kb 267659 303026 113.21

Мы можем попробовать другой способ бенчмаркинга. Как насчёт большего количества итераций, например, запуск на час? Кроме того, вместо того чтобы брать лучший результат, давайте сравним среднее значение по каждому эксперименту. Это может дать более стабильные цифры. Что вы думаете по этому поводу?

6

В моих pull request’ах для плагинов вы заметите, что многие исправления касаются поиска в глобальном пространстве имён.

Я изменил код вида

module ::Jobs
  class TranslatorMigrateToAzurePortal < Jobs::Onceoff

на

module ::Jobs
  class TranslatorMigrateToAzurePortal < ::Jobs::Onceoff

Одна вещь меня беспокоила в этом решении: почему это работало до Zeitwerk? Вопрос о том, почему что-то работает, хотя не должно, всегда непрост :slight_smile:

Я думаю, что нашёл возможный ответ в описании классического автозагрузчика (https://guides.rubyonrails.org/autoloading_and_reloading_constants_classic_mode.html#resolution-algorithms) — «Если не найдено, то алгоритм поднимается вверх по цепочке предков cref».

Zeitwerk работает более строго. Как только я попытался загрузить код до исправления, он жаловался, что Jobs::Jobs::Onceoff не может быть найден.

Сэм предложил в pull request FIX: Use top-level namespace for base classes · discourse/discourse-prometheus-alert-receiver@ef9c238 · GitHub, что вместо использования < ::Jobs::Onceoff мы можем просто использовать < Onceoff, и он прав. Я проверил, что без пространства имён это тоже работает. Я думаю, что указание :: явно говорит о том, что мы наследуемся от класса ядра Discourse, однако мы можем выбрать любой из этих вариантов.

7

Я считаю, что когда код расположен так близко друг к другу, это выглядит отлично:

module ::Jobs
  class TranslatorMigrateToAzurePortal < Onceoff

Если же код начинает «расползаться» вниз, то явное указание становится более логичным… например:

module ::Jobs
  [ 50 строк пропущено]
  class TranslatorMigrateToAzurePortal < ::Jobs::Onceoff

Тем не менее, я колеблюсь в этом вопросе, поэтому меня устраивает любой вариант. ::Jobs::Onceoff достаточно короткое и предельно явное, так что пока можно остановиться на нём.

8

Я бы с радостью начал процесс слияния. @kris.kotlarek, находится ли это сейчас в состоянии, пригодном для слияния? Время очень удачное, так как у нас только что завершилась бета-версия.

10

Давайте я перебазуирую недавний master в эту ветку и проверю, что всё ещё работает. Я сделаю это сегодня.

11

@sam, думаю, мы готовы. Я сделал ребейз с актуальной веткой master и внес небольшое исправление в Webauthn.
Я проверил три вещи:
запустил сервер локально и немного потыкал, чтобы убедиться, что всё работает как ожидается
запустил тесты (specs)
загрузил все официальные плагины и убедился, что их тесты проходят (сначала нужно слить корректировки для плагинов)

12

Круто, я сливаю это. Посмотрим, что получится!

13

Можно ли также объединить исправления для плагинов? В противном случае, если вы попытаетесь запустить Discourse с плагинами, это не сработает

14

Пожалуйста, смело сливайте!

16

Это уже слито! Если авторы плагинов испытывают трудности, сообщите нам об этом в отдельной теме!

Отличная работа, Крис!!!