4

For a while I've been having difficulty handling large navigation in craft. I built this site a while ago, and if I am honest I would probably change the structure if I was doing it again, but that's for another time.

The nav is stored in a structure and each navigation entry is either linked to an entry or a custom url is entered. There are approximately 300 pages in the entire nav.

Here is the code I use to generate the nav:

{% macro navigation(pages, id='') %}
    <nav role="navigation">
        <ul role="menu" {% if id|length %} id="{{ id }}" {% endif %}>
        {% nav node in pages %}
            {% set url          = node.url|length ? node.url : node.linkedEntry|length ? node.linkedEntry.first().url : node.customUrl %}
            {% set active       = node.slug == craft.request.getSegment(node.level) %}
            {% set class        = node.children|length ? "has-children" %}
            {% set class        = active ? class ~ " active" : url == craft.request.url ? class ~ " active" : class %}
            {% set separator    = node.separator  ? 'separator' %}

            <li class="{{ class is not empty ? class  : "" }}{{ separator }} level-{{ node.level }}" >
               <a href="{{ url }}">
                   <span>{{ node.title }}</span>
               </a>

                {% ifchildren %}
                    <ul class="children">
                        {% children %}
                    </ul>
               {% endifchildren %}
            </li>
        {% endnav %}
        </ul>
    </nav>
{% endmacro %}

The problem I have is that this is incredibly slow, in some cases I see load times of over 10s, which is unacceptable! I know that this is caused by the nav and not the rest of the page content.

I use caching to speed rendering up, and this works fine, but that only really masks the problem.

I reckon that the bulk of the time is taken up checking whether it is active, what class it has etc. Is it possible to store it as a blob and run some kind of regex match on it as a whole?

I'd prefer it if the user could change the nav through the CMS, but if I am honest, this is not a necessity. I've even thought about hardcoding it!

Any one have any pointers or experience in the past?

4

It's those two lines that trigger database queries

{% set url = node.url|length ? node.url : node.linkedEntry|length ? node.linkedEntry.first().url : node.customUrl %}
{% set class = node.children|length ? "has-children" %}

The first one can be optimized by using the first method in the ternary condition. You just need to know if there is a linked entry, not how many. node.linkedEntry|length queries for all matching entry models and is slower than just calling node.linkedEntry.first(). So change it to

{% set url = node.url|length ? node.url : node.linkedEntry.first() ? node.linkedEntry.first().url : node.customUrl %}

Same applies to the node.children criteria model in the second line. Change it accordingly

{% set class = node.children.first() ? "has-children" %}

That's about all you can do to optimize the code, now you should definitly add caching to it. It's not as straightforward to implement in this situation, as there's the dynamic "active" class you want to add to the output. So you have to either cache the whole navigation for each page it's used individually (each with the active class at the right place) or you use the string replacement technique described in this SE to cache the navigation globally for all pages.

|improve this answer|||||
  • Brilliant, thank you. I hadn't really thought of using .first(). The method you suggested for the active class is exactly what I was after. Cheers – Kyle Jan 28 '16 at 14:19
2

Carl's answer is very good and offers some quick performance wins.

I just want to add a couple of things:

First – I think its wrong to think of caching as "just masking the problem". In a situation where you're still running into performance issues and long load times after optimizing your code as much as possible (and there isn't another way to design or model the content), caching is actually the only viable solution – apart from significantly upgrading your server hardware – in terms of getting better performance.

Second – even if you can shave those 10 seconds off of the pageload by caching your navigation, you need to remember that every time an entry is updated, deleted or added to the navigation Structure, Craft will clear the cache (as it should). This means that the first person visiting the site after the navigation is updated will experience the extremely long pageload. You can combat this by implementing cache warming, which basically means that you rebuild the cache immediately after clearing it.

|improve this answer|||||
  • Thanks for the feedback. The reason I worded it as "masking the problem", is that I knew that the code could be optimised, I just wasn't entirely sure where and how. Caching worked very well (and still does), so I will absolutely continue to use it. I will look into cache warming, thank you. – Kyle Jan 28 '16 at 15:14

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.