Skip to content

Write raw tags directly to buffer#135

Merged
peterzhu2118 merged 1 commit into
masterfrom
pz-raw-tag-tokenizer
Jan 4, 2021
Merged

Write raw tags directly to buffer#135
peterzhu2118 merged 1 commit into
masterfrom
pz-raw-tag-tokenizer

Conversation

@peterzhu2118
Copy link
Copy Markdown
Contributor

Write raw tags directly to the instructions buffer as an OP_WRITE_RAW instruction. This is an alternative to #134.

Benchmarks

There is a small performance hit during parsing, but a slight performance improvement during rendering in the microbenchmark.

Master:

               parse    105.637k (± 6.9%) i/s -    535.392k in   5.092843s
              render    206.248k (± 8.5%) i/s -      1.036M in   5.058648s
      parse & render     57.954k (± 6.6%) i/s -    292.464k in   5.069258s

This branch:

               parse     97.280k (± 3.3%) i/s -    486.423k in   5.006012s
              render    225.835k (± 6.2%) i/s -      1.126M in   5.009018s
      parse & render     53.268k (± 8.9%) i/s -    268.041k in   5.077031s

Script:

require "benchmark/ips"
require "liquid"
require "liquid/c"

source = <<~LIQUID
  {% raw %}
    {% test %}
    hello world
    {% endtest %}
  {% endraw %}
LIQUID

template = Liquid::Template.parse(source)

Benchmark.ips do |x|
  x.report("parse") do |times|
    times.times do
      Liquid::Template.parse(source)
    end
  end

  x.report("render") do |times|
    times.times do
      template.render!
    end
  end

  x.report("parse & render") do |times|
    times.times do
      Liquid::Template.parse(source).render!
    end
  end
end

Copy link
Copy Markdown
Contributor

@macournoyer macournoyer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this one is better then 134 & Shopify/liquid#1378 on all fronts. It's simpler, faster, and since the goal it to move to byte-code, I don't see why we would do it any other way?

Copy link
Copy Markdown
Contributor

@dylanahsmith dylanahsmith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense as an approach before #96 is merged

@peterzhu2118 peterzhu2118 merged commit bd5ef13 into master Jan 4, 2021
@peterzhu2118 peterzhu2118 deleted the pz-raw-tag-tokenizer branch January 4, 2021 20:42
dylanahsmith pushed a commit that referenced this pull request Feb 12, 2021
Write raw tags directly to buffer

(cherry picked from commit bd5ef13)
dylanahsmith pushed a commit that referenced this pull request Mar 17, 2021
Write raw tags directly to buffer

(cherry picked from commit bd5ef13)
@shopify-shipit shopify-shipit Bot temporarily deployed to rubygems February 8, 2022 20:25 Inactive
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

Successfully merging this pull request may close these issues.

3 participants