Pelican-Elegant / elegant

Best theme for Pelican Static Blog Generator
https://elegant.oncrashreboot.com/
MIT License
293 stars 187 forks source link

Double loop in template causes performance degradation and *thousands* of `slugify` calls #145

Open mlissner opened 9 years ago

mlissner commented 9 years ago

I use Elegant for my blog and I have about 300 posts. With this many posts, generating my content was getting slow, so I took a moment to do some profiling. Turns out that when I generate my site, slugify is called about 300,000 times!

The reason, as identified by one of the Pelican contributors is a double for loop in Elegant.

According to the Pelican contributor, it looks like this could be made much faster by changing the code from:

if aTag == tag

to:

if aTag.slug == tag.slug

Seems like a simple fix that shouldn't produce any unintended consequences.

iranzo commented 5 years ago

I've tried manually applyng the change on my deployment:

diff --git a/themes/elegant/templates/article.html b/themes/elegant/templates/article.html
index 7b63b8eb..146e2f84 100644
--- a/themes/elegant/templates/article.html
+++ b/themes/elegant/templates/article.html
@@ -95,7 +95,7 @@
             <ul class="list-of-tags tags-in-article">
                 {% for tag in article.tags|sort %}
                 <li><a href="{{ SITEURL }}/{{ TAGS_URL }}#{{ tag.slug }}-ref">{{ tag }}
-                    {% for aTag, tagged_articles in tags if aTag == tag %}
+                    {% for aTag, tagged_articles in tags if aTag.slug == tag.slug %}
                     <span>{{ tagged_articles|count }}</span>
                     {% endfor %}</a></li>
                 {% endfor %}

But in my case the total generation time didn't varied (I don't have 300 posts as you)

If this is still relevant should be an easy merge

silverhook commented 5 years ago

I have 300+ posts and my server is a little ARM board, so ideal for testing bottlenecks ;)

Happy to test it out :)

silverhook commented 5 years ago

The results are in!

Before the change:

Done: Processed 367 articles, 6 drafts, 2 pages and 2 hidden pages in 368.62 seconds.

After the change:

Done: Processed 367 articles, 6 drafts, 2 pages and 2 hidden pages in 360.82 seconds.

At least in my case, it doesn’t seem to change much. Does anyone have a different experience with it?

Perhaps we just need to implement more from https://github.com/getpelican/pelican/issues/1493

iranzo commented 5 years ago

At least it doesn't break anything, so it might make sense to implement something like this ahead of the commits they mention on pelican

silverhook commented 5 years ago

@mlissner, did you try implementing the other stuff that they mention in getpelican/pelican#1493 ?

mlissner commented 5 years ago

Um, I tried everything I said I tried in that ticket, yeah? But that was four years ago.

iranzo commented 5 years ago

Perhaps pelican has been optimized, but still should be relevant to use the new approach even if there's some seconds in difference

talha131 commented 5 years ago

To the best of my knowledge, the nested loop is to be blamed for the performance degradation. The nested loop has a complexity of O(N^2).

Compare the code in articles page with the one on tags page.

Loops are used to calculate number of articles that have a particular tag. What we can do is

  1. Submit patch to pelican that adds a count property to the tag
  2. Create a pelican-plugin, that does the counting for you, which later theme can use

Existing props of tag object can be viewed here.

Then use it inside theme to show the count. So this solution to this issue can be broken down into two parts.

  1. Submit patch, and get it approved, to either pelican or pelican-plugins
  2. Change code in theme
silverhook commented 5 years ago

I really think optimisations are a great thing to have and we should implement that.

But IMHO this should not be a requirement for 2.0. If we manage to get it done before everything else is done for 2.0, awesome! If we’ll still need to work for it, it should just get into the next version.

talha131 commented 5 years ago

Right. I have moved it to milestone 3.0.

silverhook commented 3 years ago

Any progress here?

iranzo commented 3 years ago

Any progress here?

Not that I'm aware, if you think that this can be fixed, feel free to submit a PR so that we can get it merged ASAP in next branch

I've been lately merging some of the ones that we had there and pinging submitters for both issues and pr's

silverhook commented 3 years ago

Fixing something like this is waaaay out of my league. I merely volunteered to test it, since I’m running Pelican on an ARM board, so any performance changes show really well there.