Liquid profiler #3762
Liquid profiler #3762
Conversation
|
It's useful to me, I have several very huge sites that would benefit from this because I know they have some pain spots I just don't have time to figure them out in hand-to-hand combat. I'm definitely 👍 |
|
One thing that is not ideal: I'm generating the profile every time, not just when the |
|
The question isn't whether it generates the profile every time it's whether that impacts build times with incremental regeneration. It would be a deal break if it did but that should be easy to resolve by just modifying the flag to either start it or not start it no? |
|
I don't expect this to have much overhead, really.. But I can look into it more if @parkr thinks that this is worthwhile. |
|
I love everything about this idea. |
|
Any comments about the implementation? |
|
Would be helpful if you guys could run this on some of your sites to get some confidence in this working correctly. |
|
I won't be able to help with that for a day or so, busy working on other stuff :/ |
|
I like the profiler. @fw42 could you please align the file names to the left? |
|
Gonna agree that the name on the left and everything else on the right is more readable. |
|
I made the filenames left-aligned, cleaned up the code a little bit, and added a very basic test. I'm ok with this being merged now as is, assuming someone could confirm that this works as expected on their sites (I tried the Jekyll site and my own site so far). |
|
👍 This is great! It'll be super useful for users with larger sites. |
|
I'm loving it too, I just played with it for our sites and found a few bottlenecks. Left a few notes though. |
|
Addressed your comments. Thanks for the feedback. |
|
Also added a counter for number of generated bytes of rendered output, looks like this now: In my experience, that number of bytes is usually a good indication of why things are slow (huge nested loops generating megabytes of whitespace, for example). |
|
What does it mean when I see a post with |
|
What is the source of the page? |
Sorry, it's not public. If I'm the only one seeing this, I can just chalk it up to some plugin doing strange things. It's just that the word |
|
Turns out, all posts seem to generate this, it just doesn't appear most of the time. If you create a Jekyll site that has nothing but a post or two, each post will show up in the profiler twice, once with |
|
Hm interesting. I have no idea what that is. Is that a Jekyll internal thing where it modifies the path or something? I can merge those two if that makes sense. Or does it make more sense to show them separately? |
|
There is the excerpt piece to the front-matter that's shipped to you isn't there? |
|
I don't understand why "#excerpt" appears as the filename though unless it's overwriting the Post's path as well or something like that. Anyway, should I merge those two or keep them as separate ones? |
We do that so you know it's the post's excerpt, not the excerpt itself. So for @pathawks's example, the excerpt is the entire post instead of just a snippet of it. That would be a red flag for me.
Keep them as separate, please! |
|
Are users penalized for not having an "excerpt" because none of my sites provide an excerpt because I have no use for them (because we take advantage of canonical urls and meta) to just post the full article and inform Google/Bing that this is just an alias of post. |
None of my posts have an excerpt, and none of my layouts try to use an excerpt. If Jekyll is really spending double the time for each post to generate the excerpt (once for But this is pretty far off topic, I suppose. |
|
Anything left to do here? Everybody happy with the code? Seems like a few people have tried this out on their site. Any problems so far? |
|
👍 |
|
|
| def print_stats | ||
| if @config['profile'] | ||
| puts @liquid_renderer.stats_table | ||
| end |
There was a problem hiding this comment.
why not use a suffix if statement here?
There was a problem hiding this comment.
Personally I think this way is more readable, even if it takes up 2 more lines. I do the same exact thing he did in methods like that. I only suffix the if when there are more lines to follow but if it's a boolean method like this on I will multi-line the boolean statement.
|
Eggcellent. 🥚 |
|
❤️ |
|
Can't wait to use the profiler.
|
|
@penibelst if you don't want to wait you can use |
|
@envygeeks Thank you.
|
|
It seems to me that testing didn't point this out (I should have probably used it on a production site) but today when the new beta was released I ran into a problem, this seems to be passing data that was previously never passed? And the source of that file: https://github.com/envygeeks/ruby-content-pipeline/blob/master/lib/content/pipeline/jekyll/converter.rb |
|
Nevermind, I blamed my site and that file and it turns out this is not to blame, somebody adjust data. |
Sent from Yahoo Mail on AndroidFrom :"Pat Hawks" < notifications@github.com > |
This PR adds a
--profileflag to Jekyll which generates a Liquid performance profile. I found this very helpful when trying to figure out why our site was so slow and which pages are the most worthwhile to optimize.The output shows the number of times a certain file is being rendered and the total time it took to render that file (only the 50 worst offenders are shown).
Looks like this on the Jekyll site:
Doesn't look very impressive on this small site that is already quite fast, but I found some nice improvements with this on our bigger site.
The code that pretty-prints the output is a little bit ugly. Suggestions welcome about how to improve it.
@parkr @envygeeks @alfredxing, do you think this is something that could be useful for people?
@csaunders FYI