sighmon / mjml-rails

MJML + ERb/Haml/Slim view template
https://mjml.io
Other
300 stars 65 forks source link

Slow boot time due to version check #125

Closed danieldiekmeier closed 1 week ago

danieldiekmeier commented 1 month ago

I profiled the boot time of our medium sized Rails application with Vernier. I used these instructions for instrumentation: https://test-prof.evilmartians.io/profilers/ruby_profilers?id=profiling-application-boot-with-vernier and uploaded the result to https://vernier.prof.

I was surprised to see that ~80% of the boot time (850ms of total 1200ms) was taken up by mjml-rails:

image

The reason I found for this is that rails-mjml searches for the binary AND checks its version on every boot. I understand that this can be useful sometimes, and can prevent some problems, but it adds almost a second of penalty for every time I boot the app or run a single RSpec test. I also understand that the current check works this way to be as independent of configuration differences as possible.

As a workaround, I added an initializer that sets the custom binary and, more importantly, overrides the check_for_custom_mjml_binary to avoid the use of Open3.

# config/initializers/mjml.rb

# mjml-rails checks the version of the mjml node_module on every boot, in multiple different ways.
# This is incredibly slow: It adds ~700ms of boot time every time, since it runs the mjml CLI and
# checks the `--version` output: https://github.com/sighmon/mjml-rails/blob/75102ad7b8344e6dc0e86f3c0db4b70fd926de5f/lib/mjml.rb#L37-L46
# We can circumvent this since we know we're using yarn and where the binary will be. 
# We can also read the package.json instead of running the CLI to check the version compatibility.

Mjml.mjml_binary = Rails.root.join("node_modules/.bin/mjml")

def Mjml.check_for_custom_mjml_binary
  package_json = JSON.parse(Rails.root.join("node_modules/mjml/package.json").read)
  if package_json["version"].start_with?(Mjml.mjml_binary_version_supported)
    mjml_binary
  end
rescue
  false
end

Maybe this "naive" check could be added as a "99% happy path" shortcut version before going the slow route of asking each package manager? I expect most Rails users probably use yarn/npm and their mjml binary is in the node_modules folder.

sighmon commented 1 month ago

@danieldiekmeier That sounds like a good plan to me, would you like to put together a PR?

danieldiekmeier commented 1 week ago

@sighmon Sorry for the delay, but yes I would! It's over here: #127. Thank you for being open to this! :)