Slow for loops - Githubissues

chrismytton commented 8 years ago

I've been running into some performance issues with for loops being slow when iterating over lots of items. It seems that even when the for loop has no body it's still quite slow.

I've put together a benchmark which reproduces this problem with some real-world data I've been working with. https://gist.github.com/chrismytton/a34ace5123c27e11dd55

parkr commented 8 years ago

Was just running an object stackprof of Jekyll and noticed the for loop creates a lot of object:

   5335880 (145.3%)       39984   (1.1%)     block (2 levels) in Liquid::For#render

From:

$ cd jekyll
$ export BENCHMARK=true
$ bundle install
$ script/stackprof object
==================================
  Mode: object(100)
  Samples: 3671823 (0.00% miss rate)
  GC: 0 (0.00%)
==================================
     TOTAL    (pct)     SAMPLES    (pct)     FRAME
    801771  (21.8%)      801771  (21.8%)     Jekyll::URL#sanitize_url
    358200   (9.8%)      358200   (9.8%)     Jekyll::Utils#add_permalink_suffix
    492668  (13.4%)      314216   (8.6%)     Jekyll::URL#generate_url_from_drop
   1954884  (53.2%)      148840   (4.1%)     Jekyll::Document#url
  12339233 (336.1%)       88465   (2.4%)     Liquid::Block#render_all
   1986808  (54.1%)       85815   (2.3%)     Jekyll::Drops::Drop#[]
    503967  (13.7%)       74127   (2.0%)     Jekyll::Collection#url_template
    429840  (11.7%)       71640   (2.0%)     block in Jekyll::Collection#url_template
     71183   (1.9%)       71183   (1.9%)     Liquid::Variable#taint_check
    214776   (5.8%)       55921   (1.5%)     Kramdown::Parser::Kramdown#parse_spans
    178452   (4.9%)       50287   (1.4%)     block in Jekyll::URL#generate_url_from_drop
     48992   (1.3%)       48992   (1.3%)     #<Module:0x007fc36b4b8fd0>.sanitized_path
     44743   (1.2%)       44743   (1.2%)     Kramdown::Utils::Html#escape_html
     63392   (1.7%)       43122   (1.2%)     Kramdown::Utils::Html#html_attributes
   5335880 (145.3%)       39984   (1.1%)     block (2 levels) in Liquid::For#render
   1276891  (34.8%)       39384   (1.1%)     Liquid::Variable#render
     39883   (1.1%)       39091   (1.1%)     Liquid::VariableLookup#initialize
     38515   (1.0%)       38515   (1.0%)     Jekyll::Document#respond_to?
     91935   (2.5%)       34611   (0.9%)     Kramdown::Parser::Kramdown#parse_list
     34362   (0.9%)       34362   (0.9%)     Kramdown::Element#initialize
     46263   (1.3%)       30842   (0.8%)     Kramdown::Parser::Base#add_text
    575339  (15.7%)       30840   (0.8%)     Kramdown::Parser::Html::Parser#handle_html_start_tag
     33180   (0.9%)       30753   (0.8%)     Kramdown::Parser::Html::Parser#parse_html_attributes
     50608   (1.4%)       30066   (0.8%)     Jekyll::URL.escape_path
     86315   (2.4%)       30006   (0.8%)     Kramdown::Parser::Kramdown#parse_link
     33375   (0.9%)       29916   (0.8%)     Kramdown::Parser::Kramdown#parse_codespan
     29436   (0.8%)       29292   (0.8%)     Sass::CacheStores::Base#retrieve
    780744  (21.3%)       27606   (0.8%)     Kramdown::Parser::Kramdown#update_tree
    484010  (13.2%)       26278   (0.7%)     Kramdown::Parser::Html::Parser#parse_raw_html
     75065   (2.0%)       24777   (0.7%)     Liquid::Variable#lax_parse

fw42 commented 8 years ago

https://github.com/Shopify/liquid/pull/681 should make this at least a little bit better.

parkr commented 8 years ago

@fw42 You rock! Thank you so much! Do you know when 4.0 will be released?

fw42 commented 8 years ago

@parkr: No idea.. Are you waiting on that? FWIW I tried earlier today getting Jekyll current master to pass tests with Liquid current master and only a handful of tests failed (easy to fix with a one-line change)... So I don't think it's gonna be that big of a deal for you to bump (assuming your test coverage is reasonable).

parkr commented 8 years ago

Are you waiting on that?

We love to be as close to master HEAD as possible just so there aren't annoying upgrading bugs. Incremental is always better for us.

I don't think it's gonna be that big of a deal for you to bump (assuming your test coverage is reasonable).

Yay! I guess there's another change to Tag.parse?

Shopify / liquid

Slow for loops #678