Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
nflorentin opened this issue Dec 1, 2020 · 20 comments
Closed

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

nflorentin opened this issue Dec 1, 2020 · 20 comments

Comments

@nflorentin
Copy link

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
Copy link

doits commented Dec 1, 2020

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 mjmlio/mjml#794 for more info about it.

There is #52 here with the same topic btw.

@doits doits mentioned this issue Dec 1, 2020
@nflorentin
Copy link
Author

nflorentin commented Dec 2, 2020

@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:

  • take the code of the mjml email template
  • replace manually the <%= yield %> of the mjml layout with the content of the mjml email template
  • write the complete rendered email into a tmp file
  • run mjml compilation on the tmp file and write the html.erb file into a new file

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
Copy link
Owner

sighmon commented Dec 19, 2020

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

@nflorentin
Copy link
Author

nflorentin commented Jan 12, 2021

@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
Copy link

bcackerman commented Mar 30, 2021

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
Copy link

Just reverted versions to 4.4 again and immediate improvement
image

@sighmon
Copy link
Owner

sighmon commented Apr 2, 2021

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

@bcackerman
Copy link

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

@doits
Copy link

doits commented Apr 20, 2021

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
Copy link

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

@doits
Copy link

doits commented Apr 26, 2021

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
Copy link

PeteTheSadPanda commented Apr 26, 2021

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
Copy link

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

@doits
Copy link

doits commented Apr 26, 2021

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
Copy link

doits commented Aug 12, 2021

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
Copy link

arvida commented Oct 1, 2021

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
Copy link
Author

nflorentin commented May 17, 2022

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
Copy link

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
Copy link
Owner

sighmon commented Sep 10, 2022

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

@alanhala
Copy link

I created #98 which removes the need of specifying the binary manually.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants