Skip to content
This repository was archived by the owner on Jul 21, 2019. It is now read-only.

improves template rendering performance #644

Merged
merged 1 commit into from
Aug 27, 2018

Conversation

tgross
Copy link
Contributor

@tgross tgross commented Aug 25, 2018

For #643

Reduce the overall rendering time by ~75% via the following:

  • cache the entirely-static footer partial
  • eliminate looping over all talks in speaker page
  • eliminate looping over all speakers in talk page

This change is Reviewable

reduce the overall rendering time by ~75% via the following:
- cache the entirely-static footer partial
- eliminate looping over all talks in speaker page
- eliminate looping over all speakers in talk page
@bridgetkromhout
Copy link
Contributor

cache the entirely-static footer partial

I'm not sure if I understand entirely how this cache would interact with the Twitter widget in the footer. If the Twitter box can't update until our site builds again, that wouldn't be ideal.

@bridgetkromhout
Copy link
Contributor

eliminate looping over all talks in speaker page
eliminate looping over all speakers in talk page

This is super cool. I want to take a little time with this to make sure I can't break it (muhahaha) and @mattstratton is unavoidably AFK until probably Thursday, but I should be able to get this tested enough to merge before that (and he can do a release after that).

@mattstratton
Copy link
Member

Thanks @tgross! This is all so awesome. I’m sure there a lots of examples where I was inefficient. Thank you so much!!

@mattstratton
Copy link
Member

cache the entirely-static footer partial

I think that just means the evaluation in the partial isn’t run when building every single page since it’s identical in every single page.

@bridgetkromhout
Copy link
Contributor

Thanks, @tgross! I ran this locally, and you're totally right about the speedup; the currently- code finishes in Total in 101777 ms while this branch finishes in Total in 29991 ms.

And (as expected) it produces the desired results. Hooray!

@bridgetkromhout bridgetkromhout merged commit 0109891 into devopsdays:master Aug 27, 2018
@tgross
Copy link
Contributor Author

tgross commented Aug 27, 2018

I'm not sure if I understand entirely how this cache would interact with the Twitter widget in the footer. If the Twitter box can't update until our site builds again, that wouldn't be ideal.

Obviously we've merged this at this point but just in case someone else is reading this: the Twitter widget is client-side in Javascript so we can safely pass the same script to everyone and it'll be up-to-date.

tgross added a commit to tgross/devopsdays-theme that referenced this pull request Sep 13, 2018
The performance improvement in devopsdays#644 to the speaker page implicitly
(and incorrectly) assumes that a speaker will give a single talk
per event. Rather than using .GetPage which can return only a single
page, we can change the original call to .Site.Pages and get the
correct behavior with at least some of the performance improvement.

This could be further improved if the program YAML included the event
name, or if Hugo supported a "contains" or "startswith" operator in
the where function.

Fixes devopsdays#643 (comment)
Replaces devopsdays-web f944fea71d071b959f715e9cdcb0ebb00f52c978
tgross added a commit to tgross/devopsdays-theme that referenced this pull request Sep 13, 2018
The performance improvement in devopsdays#644 to the speaker page implicitly
(and incorrectly) assumes that a speaker will give a single talk
per event. Rather than using .GetPage which can return only a single
page, we can change the original call to .Site.Pages and get the
correct behavior with at least some of the performance improvement.

This could be further improved if the program YAML included the event
name, or if Hugo supported a "contains" or "startswith" operator in
the where function.

Fixes devopsdays#643 (comment)
Replaces devopsdays-web f944fea71d071b959f715e9cdcb0ebb00f52c978
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants