sighmon / mjml-rails

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

Email very slow to be sent: compilation time is the cause ? #76

Closed nflorentin closed 2 years ago

nflorentin commented 3 years ago

Hi and thank you for this gem!

I have one question: is there a possibility to compile only once the mjml to html and not everytime an email is sent ?

I'm seeing an increase from 10ms to 1000 to 2000ms to send an email. I imagine that the cause is the mjml compilation.

Thank in advance for your help!

doits commented 3 years ago

Sending e-mails takes the same amount of time for me, so this seems to be normal.

I don't think precompilation is possible though since MJML must work on the complete body – it cannot compile partials for example, because output depends on stuff around it like head components. So it must know the complete content beforehand, which in fact means it must be compiled once for every e-mail sent. I'm no MJML expert though (only a user of it), so maybe I am wrong.

See also https://github.com/mjmlio/mjml/issues/794 for more info about it.

There is #52 here with the same topic btw.

nflorentin commented 3 years ago

@doits Thanks for you answer!

You are right, after a bit of digging, I arrive to same conclusion. Erb rendering has to happened prior to the mjml rendering to render partials and the email template inside the layout. It would be impossible to switch the order.

I thought of a trick if the slow perf is really a problem for someone, making a rake task which:

The last file will be seen by the app like a normal html.erb template. The problem with that approach is you won't be able to render partials with erb in your email template, unless you don't mind them to not be compiled with mjml (so they would be necessarily mj-raw if I understood it well).

Another possibility if you really need to be able to render partials is to use <mj-include> tags.

sighmon commented 3 years ago

@nflorentin I just merged @doits latest PR which optimises the regex, so see if that makes any significant difference for you.

nflorentin commented 3 years ago

@sighmon Sorry for the delay !

I tried it, there is a 5-10% decrease in compilation time (the compilation time is ~975ms instead of 1050ms for a small email in my app).

Thanks for your feedback.

bcackerman commented 3 years ago

Also seeing the same with upgrading from 4.4 to 4.6.1

Dark blue line is rendering time for emails with 4.6.1, the dotted grey line is from 4.4 image

bcackerman commented 3 years ago

Just reverted versions to 4.4 again and immediate improvement image

sighmon commented 3 years ago

@bcackerman Would you mind trying 4.5 as well to see which version introduced the higher compilation time?

bcackerman commented 3 years ago

4.5 works as normal! Seems it's 4.6.1 that's the issue

doits commented 3 years ago

Can you please test with 4.6.0, too?

And can you check if those two setting make a difference on latest version?

Mjml.setup do |config|
  config.beautify = false
  config.validation_level = "skip"
end
PeteTheSadPanda commented 3 years ago

Hey @doits these configuration values haven't helped, still very slow processing unrelated to swap or any such thing.

doits commented 3 years ago

Is it same for you that one version is fast and the next one is slow? If so, which last version is fast and which one the first one to be slow?

PeteTheSadPanda commented 3 years ago

4.4 was the last version which was performant, each version we've tried beyond that has been what appears to be an order of magnitude slower. Worth noting, we are hosted on heroku and the version of mjml as installed by npm globally is mjml-core@4.9.0, we are using version 4.6.0 of this gem

PeteTheSadPanda commented 3 years ago

Apologies @doits I believe it was 4.5 (Bruce and I are on the same team with the same issue).

doits commented 3 years ago

Just to be sure and explicit: 4.6.0 (with a zero) is the first slow version?

Another wild guess: Can you please see on latest version if explicitly providing the path to the binary?

# config/initializers/mjml.rb
Mjml.setup do |config|
  config.mjml_binary = "/path/to/mjml"

  # or if you use yarn
  config.mjml_binary = "yarn run mjml"
end

... and if that does not solve it one last try: Please redefine valid_mjml_binary to directly return it:

# config/initializers/mjml.rb
require 'mjml-rails/mjml'

module Mjml
  def self.valid_mjml_binary
    # to see that it is indeed overwritten and used
    puts "I'm the overwritten valid_mjml_binary"

    "/path/to/bin/mjml"
  end
end

Does any of these help?

doits commented 3 years ago

Just another heads up if you use yarn: yarn run is really slow for me and calling mjml by ./node_modules/.bin/mjml is much faster

→   time yarn run mjml --version                                                                                                                        [fix_some_hidden_stuff]
mjml-core: 4.10.2
mjml-cli: 4.10.2
yarn run mjml --version  1,26s user 0,27s system 123% cpu 1,238 total

→   time ./node_modules/.bin/mjml --version                                                                                                             [fix_some_hidden_stuff]
mjml-core: 4.10.2
mjml-cli: 4.10.2
./node_modules/.bin/mjml --version  0,38s user 0,18s system 103% cpu 0,539 total

So you could try to specify mjml custom binary to not use yarn run and see if this speeds up thing.

Yarn 3 might have fixed it, as they write in their changelog https://github.com/yarnpkg/berry/blob/master/CHANGELOG.md:

yarn run should be significantly faster to boot on large projects.

arvida commented 2 years ago

Just another heads up if you use yarn: yarn run is really slow for me and calling mjml by ./node_modules/.bin/mjml is much faster …

We just started to do this (using the mjml binary directly by setting mjml_binary in Mjml.setup). And it looks like it has removed around 300 ms on average on our e-mail rendering time on Heroku. I only have data for a couple of hours yet, but it looks promising.

nflorentin commented 2 years ago

I did some tests.

Defining the binary as ./node_modules/.bin/mjml seems to give a 45% decrease in compilation time. Updating to 4.6.1 (regexp improvement) gives an additional 4-5% decrease.

adipasquale commented 1 year ago

thank you for the tip, this massively sped up mail compile times for me:

# config/initializers/mjml.rb
Mjml.setup do |config|
  config.mjml_binary = Rails.root.join("node_modules/mjml/bin/mjml")
end

I was actually looking at trying mrml a much faster rust implementation of MJML because it was so slow with the default mjml-rails setup. This config tip has improved the performances so much that I'll stop looking. thanks!

sighmon commented 1 year ago

@adipasquale Nice one. I'll add that to the README.

alanhala commented 1 year ago

I created https://github.com/sighmon/mjml-rails/pull/98 which removes the need of specifying the binary manually.