Skip to content
New issue

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

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

Already on GitHub? # to your account

Optimizations #62

Merged
merged 4 commits into from
Nov 16, 2022
Merged

Optimizations #62

merged 4 commits into from
Nov 16, 2022

Conversation

asterite
Copy link
Contributor

@asterite asterite commented Nov 8, 2022

Fixes #40

Rendering was slow because in many places a regex was dynamically created by using interpolation. When you do that, a new Regex is created each time. Doing that in a loop makes things really slow.

This PR moves those dynamic regexes into constants.

Here's the benchmark I used:

require "benchmark"
require "./src/liquid"

def any(v)
  Liquid::Any.new(v)
end

template = <<-LIQUID
{{ order.uid }}
{{ order.total }}
{{ order.display_total }}
{% for item in order.line_items_collection %}
{{ item.offer_uid }}
{{ item.name }}
{{ item.price }}
{{ item.display_price }}
{{ item.qnt }}
{{ item.url }}
{{ item.main_image_url }}
{{ item.old_price }}
{{ item.display_old_price }}
{% if item.available %}available{% else %}unavailable{% endif %}
{{ item.description }}
{% endfor %}
LIQUID

order = Liquid::Any.new({
  "account"               => any({"id" => any(1)}),
  "visitor"               => any({"id" => any(100)}),
  "uid"                   => any("order-123"),
  "total"                 => any(1000.0),
  "display_total"         => any("$1000"),
  "line_items_collection" => any([
    any({
      "offer_uid"     => any("o-123"),
      "qnt"           => any(1.0),
      "price"         => any("50.0"),
      "display_price" => any("$50"),
    }),
    any({
      "offer_uid"      => any("o-456"),
      "qnt"            => any(2),
      "price"          => any(25),
      "name"           => any("Offer 123"),
      "display_price"  => any("$25"),
      "url"            => any("https://example.net/products/o-456"),
      "main_image_url" => any("https://example.net/images/o-456.jpg"),
      "description"    => any("A multiline\ndescription."),
    }),
  ]),
})

tpl = Liquid::Template.parse(template)
ctx = Liquid::Context.new
ctx.set("order", order)

Benchmark.ips do |x|
  x.report("parse & render") {
    ctx = Liquid::Context.new
    ctx.set("order", order)
    Liquid::Template.parse(template).render(ctx)
  }
  x.report("render") { tpl.render(ctx) }
end

Output before this PR:

parse & render 544.86  (  1.84ms) (± 0.65%)  389kB/op   2.05× slower
        render   1.12k (893.29µs) (± 1.15%)  202kB/op        fastest

Output with this PR:

parse & render  20.29k ( 49.28µs) (± 0.47%)  51.4kB/op   1.52× slower
        render  30.77k ( 32.50µs) (± 0.38%)  23.7kB/op        fastest

So about 37/27 times faster than before, respectively.

We can now tell the user complaining that Crystal here was 2.5 times slower than Ruby that Crystal is now 15 times faster than Ruby.

By the way, I created those "INTERNED_..." constants, but it looks like those regexes are exactly the same as the "interned" ones 🤔 (I don't know why "intern" is used here)

@asterite
Copy link
Contributor Author

asterite commented Nov 8, 2022

There's room for more optimizations, but this is the simplest thing that can be done (other optimizations will take me a bit more time.)

@crimson-knight
Copy link
Member

@asterite microseconds is definitely what I like to see with a crystal shards processing time!

@asterite
Copy link
Contributor Author

asterite commented Nov 8, 2022

I just pushed another small commit. Now the times are like this:

parse & render  25.55k ( 39.15µs) (± 1.30%)  49.7kB/op   1.79× slower
        render  45.77k ( 21.85µs) (± 0.98%)  22.2kB/op        fastest

So now it's 40~46 times faster than before.

There are more regexes that could be replaced with faster logic, but I think for now this is good enough.

@hugopl hugopl merged commit 8938b9d into amberframework:master Nov 16, 2022
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Why is it astonishingly slow?
4 participants