Rails 6 ships with two autoloading modes: zeitwerk and classic. In that pull request https://github.com/discourse/discourse/pull/8083 I upgraded Rails to version 6.0.0 with classic autoloader as a transitional phase. It would be interesting to try to switch to Zeitwerk.
Zeitwerk is an efficient and thread-safe code loader for Ruby. As long as the project is following naming conventions, Zeitwerk can find correct files and load them on demand or upfront without the need for any require or require_dependency. Also, it may give a small performance boost to the application according to that article Zeitwerk integration in Rails 6 (Beta 2) | Riding Rails
There are a few steps I need to do to make it work
Change the name of few classes to follow Rails naming convention. For example file, canonical_url.rb should define CanonicalUrl class instead of CanonicalURL. Similarly file ondiff.rb should define Onpdiff class instead of ONPDiff. An alternative approach would be to hook custom inflector to the project, however, I think that following convention might be a better choice - https://github.com/fxn/zeitwerk#custom-inflector
Similar to the previous point, by convention custom validations living in validations directory should be wrapped with Validations module. Besides, some validations are inheriting from EachValidator and should be accessible without namespace. I am planning to move them to separate directory and add to autoload paths.
Remove all require_dependency and ensure that the project is working
Remove all require and ensure that Discourse is working
Ensure that all plugins can access the required dependencies. I don’t know how to achieve it yet. I want to firstly make Discourse running without any plugins.
Of course, there are still plenty of unknowns to solve. I will keep you updated about progress however if you are keen to look, I started playing with that here https://github.com/lis2/discourse/commits/zeitwerk
Please let me know if you see any drawbacks of implementing Zeitwerk or you think I missed something.
I made some progress around Zeitwerk, however, I changed my approach. My original plan was to change every place where Discourse is not following the Zeitwerk filename convention. After a few fixes, I realised that this is just the tip of the iceberg and noticed that if I am going to follow that path, then pull request will be difficult to read and confidently merge to master. For example, all job classes under regular directory should have Regular namespace, same with Onceoff and Scheduled.
I decided to step back a little bit and think about a more evolutionary approach than revolutionary.
I decided that it would be better to introduce custom Inflector, which will cover all files which are not following Zeitwerk convention. The biggest benefit will be that we would be able to deploy that small change and once we are happy with Zeitwerk and we don’t have any performance downgrades, we can start fixing convention file by file in reasonable small pull requests.
I found some problems which could not be sorted by custom Inflector, so I made additional fixes to make it work.
Pull request is still in progress, however, at this stage, I can run Discourse with Zeitwerk and default plugins, run all specs and run benchmark without a problem.
I wanted first to be at that stable state when all specs are passing. Now I can confidently start removing all require_dependency one by one and also test official plugins. Once everything is ready, I will share with you benchmark results in this post.
We can try another way to benchmark. What would you say about more iterations, something which would run for an hour? In addition, instead of taking the best result, compare the average from each experiment. That may give more consistent numbers. What do you think?
@sam I think we are good to go, I rebased with the latest master and made a small adjustment to Webauthn.
I checked 3 things:
run the server on local and click a little bit to ensure it works as expected
run specs
downloaded all official plugins and ensured that specs for plugins are passing (we need to merge adjustments for plugins first)