Atualizando Discourse para Zeitwerk

Dev
1

O Rails 6 vem com dois modos de carregamento automático: zeitwerk e classic. Naquele pull request DEV: Upgrading Discourse to Rails 6 by KrisKotlarek · Pull Request #8083 · discourse/discourse · GitHub, atualizei o Rails para a versão 6.0.0 com o carregador automático clássico como uma fase transitória. Seria interessante tentar migrar para o Zeitwerk.

O Zeitwerk é um carregador de código eficiente e seguro para threads em Ruby. Desde que o projeto siga as convenções de nomenclatura, o Zeitwerk consegue encontrar os arquivos corretos e carregá-los sob demanda ou antecipadamente, sem a necessidade de qualquer require ou require_dependency. Além disso, conforme mencionado naquele artigo https://weblog.rubyonrails.org/2019/2/22/zeitwerk-integration-in-rails-6-beta-2/, ele pode oferecer um pequeno aumento de desempenho para a aplicação.

Existem algumas etapas que preciso realizar para fazê-lo funcionar:

  1. Alterar o nome de algumas classes para seguir a convenção de nomenclatura do Rails. Por exemplo, o arquivo canonical_url.rb deve definir a classe CanonicalUrl em vez de CanonicalURL. Da mesma forma, o arquivo ondiff.rb deve definir a classe Onpdiff em vez de ONPDiff. Uma abordagem alternativa seria conectar um inflector personalizado ao projeto; no entanto, acredito que seguir a convenção seja uma escolha melhor — GitHub - fxn/zeitwerk: Efficient and thread-safe code loader for Ruby · GitHub

  2. Similar ao ponto anterior, por convenção, validações personalizadas localizadas no diretório validations devem ser envoltas pelo módulo Validations. Além disso, algumas validações herdam de EachValidator e devem ser acessíveis sem namespace. Planejo movê-las para um diretório separado e adicioná-las aos caminhos de carregamento automático.

  3. Remover todos os require_dependency e garantir que o projeto esteja funcionando.

  4. Remover todos os require e garantir que o Discourse esteja funcionando.

  5. Garantir que todos os plugins possam acessar as dependências necessárias. Ainda não sei como alcançar isso. Quero primeiro fazer o Discourse funcionar sem nenhum plugin.

Claro, ainda há muitas incógnitas a resolver. Manterei você atualizado sobre o progresso; no entanto, se você tiver interesse em acompanhar, comecei a experimentar isso aqui: Commits · KrisKotlarek/discourse · GitHub

Por favor, me avise se você identificar alguma desvantagem na implementação do Zeitwerk ou se acha que deixei algo passar.

2

Fiz algum progresso em relação ao Zeitwerk, mas mudei minha abordagem. Meu plano original era alterar todos os locais onde o Discourse não segue a convenção de nomes de arquivos do Zeitwerk. Após algumas correções, percebi que isso é apenas a ponta do iceberg e notei que, se eu seguisse esse caminho, o pull request ficaria difícil de ler e de mesclar com confiança na master. Por exemplo, todas as classes de job sob um diretório regular devem ter o namespace Regular, o mesmo vale para Onceoff e Scheduled.

Decidi dar um passo atrás e pensar em uma abordagem mais evolutiva do que revolucionária.

Decidi que seria melhor introduzir um Inflector personalizado, que cobriria todos os arquivos que não seguem a convenção do Zeitwerk. O maior benefício será que poderemos implantar essa pequena alteração e, uma vez que estivermos satisfeitos com o Zeitwerk e não houver nenhuma queda de desempenho, poderemos começar a corrigir a convenção arquivo por arquivo em pull requests razoavelmente pequenos.

Encontrei alguns problemas que não puderam ser resolvidos pelo Inflector personalizado, então fiz correções adicionais para fazê-lo funcionar.

O pull request ainda está em andamento, mas, neste estágio, consigo executar o Discourse com o Zeitwerk e os plugins padrão, executar todos os specs e rodar o benchmark sem problemas.

Queria primeiro atingir esse estado estável em que todos os specs estão passando. Agora posso começar com confiança a remover todos os require_dependency um por um e também testar os plugins oficiais. Assim que tudo estiver pronto, compartilharei com vocês os resultados do benchmark neste post.

Por enquanto, se você estiver interessado no progresso, pode dar uma olhada nesse PR de rascunho: DEV: Upgrading Discourse to Zeitwerk by KrisKotlarek · Pull Request #8098 · discourse/discourse · GitHub

O arquivo mais importante é esse Inflector personalizado do Zeitwerk: DEV: Upgrading Discourse to Zeitwerk by KrisKotlarek · Pull Request #8098 · discourse/discourse · GitHub

3

Para fazer os plugins funcionarem, precisei criar mais alguns pequenos pull requests. Assim que forem mesclados, acredito que os testes no Discourse devem passar.

Também verifiquei o desempenho no Rails 6.0.0 com o Classic autoloader e no Rails 6.0.0 com o Zeitwerk.

Teste Classic Zeitwerk Porcentagem
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

Os resultados nem sempre são consistentes, então leve-os com um grão de sal.

4

A quantidade de inconsistência na mediana aqui é um pouco estranha. Fico me perguntando por que os resultados estão flutuando tanto.

Surpreso que o loader tenha algum impacto.

5

Tentei novamente, aqui estão os resultados:

Teste Clássico Zeitwerk Porcentagem
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

Podemos tentar outra abordagem para o benchmark. O que você acha de mais iterações, algo que rodaria por uma hora? Além disso, em vez de pegar o melhor resultado, compare a média de cada experimento. Isso pode fornecer números mais consistentes. O que você acha?

6

Em meus pull requests para plugins, você notará que muitas correções envolvem buscas no namespace global.

Mudei códigos como

module ::Jobs
  class TranslatorMigrateToAzurePortal < Jobs::Onceoff

para

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

Uma coisa me incomodava nessa solução: por que funcionava antes do Zeitwerk? Essa pergunta, quando algo funciona mas não deveria, é sempre complicada :slight_smile:

Acho que encontrei uma resposta potencial na descrição do carregador automático clássico (https://guides.rubyonrails.org/autoloading_and_reloading_constants_classic_mode.html#resolution-algorithms) — “Se não for encontrado, o algoritmo percorre a cadeia de ancestrais do cref”.

O Zeitwerk é mais rigoroso. Assim que tentei carregar o código antes de corrigi-lo, ele reclamava que Jobs::Jobs::Onceoff não pôde ser encontrado.

Sam sugeriu no pull request FIX: Use top-level namespace for base classes · discourse/discourse-prometheus-alert-receiver@ef9c238 · GitHub que, em vez de usar < ::Jobs::Onceoff, poderíamos simplesmente usar < Onceoff, e ele está certo. Verifiquei que, sem o namespace, também funciona. Estou pensando que usar :: é dizer explicitamente que estamos herdando de uma classe do núcleo do Discourse, no entanto, podemos seguir por qualquer um dos dois caminhos.

7

Acho que, quando o código está tão próximo assim, isso fica ótimo de ler:

module ::Jobs
  class TranslatorMigrateToAzurePortal < Onceoff

Se o código começar a ficar mais espaçado, fazer de forma explícita faz mais sentido… por exemplo:

module ::Jobs
  [ 50 linhas omitidas]
  class TranslatorMigrateToAzurePortal < ::Jobs::Onceoff

Dito isso, estou em dúvida aqui, então estou bem com qualquer uma das opções. ::Jobs::Onceoff é curto o suficiente e super explícito, então podemos seguir com isso por enquanto.

8

Gostaria muito de começar a fazer isso ser mesclado. @kris.kotlarek, isso já está em um estado mesclável? O timing é bastante bom, pois acabamos de ter uma versão beta.

10

Vou rebasear o master recente naquela branch e garantir que ainda funcione. Farei isso hoje.

11

@sam Acredito que estamos prontos para seguir. Fiz um rebase com a master mais recente e fiz um pequeno ajuste no Webauthn.
Verifiquei 3 itens:
executei o servidor localmente e cliquei um pouco para garantir que funcione como esperado
executei os specs
baixei todos os plugins oficiais e garanti que os specs dos plugins estejam passando (precisamos mesclar primeiro os ajustes para os plugins)

12

Legal, estou mesclando isso. Vamos ver no que dá!

13

Podemos mesclar correções para plugins também? Caso contrário, se você tentar executar o Discourse com plugins, ele falhará

14

Pode prosseguir e fazer o merge!

16

Isso agora está mesclado! Se algum autor de plugins estiver com dificuldades, avise-nos em um novo tópico dedicado!

Ótimo trabalho, Kris!!!