-
Notifications
You must be signed in to change notification settings - Fork 12
Build performance analysis #643
Comments
This is super exciting. Thanks for taking the time to test performance and give us such a detailed analysis, @tgross! This is going to help local development, but I also suspect it might help with our netlify build times too. |
I bet that I’m doing something similar with the program page of looping through all the talks, etc, so it probably can be made much better! |
Uh-oh! @adrianmoisey noticed that unfortunately the change in #644 had the unintended side effect of making it so when one speaker has two talks at a given event, only one of them shows up. Example: https://www.devopsdays.org/events/2018-portland/speakers/heidi-waterhouse/ - she has an ignite as well, and it shows up if I revert the changes to the speaker page that were added in this PR: Checking in with @tgross to see if it would be trivial for him to adjust that, or if in the interests of "oops, this affects an ongoing event today" I should revert those specific changes and we can revisit?
|
I would revert. The change I made relies on the URL construction to query for the correct page for the talk, but this turns out to be an incorrect assumption when there's multiple talks by the same speaker because (obviously) the two talks don't have the same URL. I'll need a bit of time to mess around with a more reliable query there, so I would recommend reverting in the meantime. |
Cool, thanks. Indeed, URL will vary - it can and will differ (of course) but also it can be a workshop name or any arbitrary string not containing speaker name. Should I revert the whole PR or just the speaker page bit, do you think? |
It's easier for me to land the fix later if you only revert the speaker page bit but probably easier for you to revert the whole commit. So whichever you want 😀 |
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
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
Using the
hugo --stepAnalysis
option we'll find that, as we'd expect, the biggest amount of time is being spent in the "render and write pages" steps (~60 seconds overall compared to millseconds for the other steps).We can use
hugo --templateMetrics
to break this down. Running this a couple times to make sure any disk cache is warmed up (although it didn't really make a difference), I get the following:The immediate low-hanging fruit here is the
partials/footer.html
which is taking up ~60s of CPU time all by itself. The footer has nothing specific to a page on it, so we can use thepartialCached
function on it. Because this is called in every page this ends up saving us almost half our rendering time right off the bat.After caching the footer:
That saves us a lot!
Some other areas of improvement to be made:
The talk and speaker pages are unique so we can't avoid rendering these a lot. But they are relatively expensive so if we can make some improvements on these it'll bring down overall time.
A look at the
talk/single.html
template shows that in order to find data about the speaker we're looping over all the speakers that have ever spoken! If instead we use the.GetPage
(with proper nil-testing), we can avoid doing all that work.After making that fix:
We do the same thing for the speaker page, where we iterate over all the talks to find the just the ones they're giving at that event. After making that fix for speaker pages too:
I've opened a PR with these three big-ticket items.
A future area to examine is the program page:
The program page is very expensive on an individual basis. So optmizing that would have a big impact.
Also for future work, smaller but harder-fought wins could be found in some of the other partials:
Probably no way around rendering most of these for each page, but each renders very quickly. If we could extract cacheable partials from these that might reduce some time here but we'd want to test each change.
cc @bridgetkromhout @mattstratton
The text was updated successfully, but these errors were encountered: