Skip to content

Implement raw tag parsing in BlockBody#134

Closed
peterzhu2118 wants to merge 1 commit into
masterfrom
pz-raw-block-body
Closed

Implement raw tag parsing in BlockBody#134
peterzhu2118 wants to merge 1 commit into
masterfrom
pz-raw-block-body

Conversation

@peterzhu2118
Copy link
Copy Markdown
Contributor

See Shopify/liquid#1378.

Benchmarks

In a microbenchmark for raw tags, this PR introduces a performance impact during parsing.

This branch:

               parse     80.202k (± 8.7%) i/s -    400.816k in   5.036587s
              render    214.678k (± 6.1%) i/s -      1.081M in   5.055158s
      parse & render     48.202k (± 5.8%) i/s -    243.744k in   5.074713s

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

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.

Looks like parsing code was simply moved to block.c 👍

I think parsing performance will soon be irrelevant with serialization 🎉

Comment thread ext/liquid_c/block.c
match->delimiter_start = NULL;
match->delimiter_len = 0;

if (len < 5) return false; // Must be at least 5 characters: \{%\w%\}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Although it is only called for raw tags at the moment, the name implies it could be used for parsing other tags. So, could it be called within {% liquid ... %}? In which case the tags would not be wrapped in {% ... %}.

@peterzhu2118
Copy link
Copy Markdown
Contributor Author

Closing since #135 is merged.

@peterzhu2118 peterzhu2118 deleted the pz-raw-block-body branch January 4, 2021 20:43
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.

2 participants