Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

simple way to speed up menus #1284

Closed
wants to merge 2 commits into from

5 participants

@evildmp
Owner

Comments etc appreciated.

@evildmp
Owner

For our site at http://medicine.cardiff.ac.uk/ with well over 1000 pages, this is very valuable - it speeds up the menus a huge amount; they're so fast they don't need to be cached.

As it was before, the menu had to cache the entire tree, and that would sometimes take six or more seconds. Even with caching, visitors would still find themselves quite often waiting a long time for a page to be loaded, and of course the cache bugged people who expected to find their changes appearing in the menus immediately.

The way this works is explained in the code.

What I don't know is whether it will work with all possible ways of generating a menu. I know that with {% show_menu 0 5 0 1 %} it does work.

Hi Evildmp,
are the testcases running with your solution?

As I do not see how this improves for logged in users, and permissions?

Thanks
Frank

Owner

From my understanding the building of the SQL statement in the get visible pages method is the time waster - isnt it?

@chrisglass
Collaborator

Clever :)

@andrewschoen

A type of menu I seem to have in every project is one that roots at a certain level (usually 0) and then shows all descendants from that tree regardless of what the active page is. So basically I'd need active page, it's descendants, it's siblings + descendants, it's ancestors and it's ancestors siblings + descendants. I hope that makes sense.

Here's an example on a recent project: http://www.rockhurst.edu/admissions/undergraduate-students/admitted-freshmen/admission-process/how-apply/

Doesn't seem like that patch would allow for this type of menu?

@andrewschoen

I wasn't able to ever get this type of rooted menu to work with show_menu, so I wrote this.

https://gist.github.com/2660259

I just need to update this to use classy tags now instead of this old school way now.

@andrewschoen

@evildmp oh, great then. You're using show_menu as well, right? Menus make my head hurt...

@andrewschoen

@evildmp just tested on that project with show_menu and it doesn't give me quite everything. In your example you gave me on that page I'd need all the descendants of 'Other Undergradute Courses' as well in that menu. Plus all other descendants of level 1 down.

If you hit the arrow signs next to a left-hand nav item on this page you'll see that I have all the descendant pages loaded into the menu. http://www.rockhurst.edu/admissions/

@evildmp
Owner
@digi604
Collaborator

Does the patch pass the testsuite?

@andrewschoen
@evildmp
Owner
@evildmp
Owner

I have a better idea! and I will submit a pull request for it soon. It's much better than this, and will make everyone happier and their lives better. So I'll close this now.

@evildmp evildmp closed this
@evildmp evildmp referenced this pull request
Closed

Truncated menu #1572

@evildmp evildmp deleted the evildmp:menu-speed-up branch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jun 12, 2012
  1. @evildmp

    simple way to speed up menus

    evildmp authored
Commits on Jun 18, 2012
  1. @evildmp

    add all roots to pages list

    evildmp authored
This page is out of date. Refresh to see the latest.
Showing with 59 additions and 0 deletions.
  1. +53 −0 cms/menu.py
  2. +6 −0 menus/menu_pool.py
View
53 cms/menu.py
@@ -234,6 +234,59 @@ def get_nodes(self, request):
pages = page_queryset.published().filter(**filters).order_by("tree_id", "lft")
+ # ------------- additional code to speed up menus -----------
+
+ # pages now contains every single page in the site
+ # it was quick to get, but some other operations are slow,
+ # especially get_visible_pages() and titles
+
+ # so why not be smarter? Instead of getting every page in the site, let's
+ # just get relevant ones, instead of ones that will never appear in the menu
+ # such as pages related only through distant ancestors. We'll get:
+ #
+ # * the current page
+ # * all of its ancestors
+ # * all of their children
+ #
+ # in order to do this we'll also need an amendment to menus/menu_pool - it's now longer
+ # enough to do:
+ #
+ # key = "%smenu_nodes_%s_%s" % (prefix, lang, site_id)
+ #
+ # we also need to cache each request_path:
+ #
+ # key = "%smenu_nodes_%s_%s_%s" % (prefix, lang, site_id, hash(request.path))
+
+ current_page = request.current_page
+
+ # get the current page as a queryset and make sure it's unique
+ current_page_queryset = pages.filter(id=current_page.id).distinct()
+
+ # get all the ancestors - a chain of ancestry back to the root
+ ancestry = current_page.get_ancestors(ascending = True)
+
+ # create a queryset of relevant pages - we use the intersection of
+ # pages & ancestry to eliminate unpublished ancestors
+ relevant_pages = pages & ancestry
+
+ # get all of:
+ # pages whose parents are in relevant pages
+ # relevant pages
+ # pages whose parents are the current page
+ # current page
+
+ pages = \
+ pages.filter(parent__in=relevant_pages).distinct() | \
+ relevant_pages.distinct() | \
+ pages.filter(parent__in=current_page_queryset).distinct() | \
+ current_page_queryset | \
+ pages.filter(level=0).distinct()
+
+ # do they need to be ordered here? I am not sure this is necessary
+ pages = pages.order_by("tree_id", "lft")
+
+ # ------------- end of additional code to speed up menus -----------
+
ids = []
nodes = []
first = True
View
6 menus/menu_pool.py
@@ -125,6 +125,12 @@ def _build_nodes(self, request, site_id):
key = "%smenu_nodes_%s_%s" % (prefix, lang, site_id)
if request.user.is_authenticated():
key += "_%s_user" % request.user.pk
+
+ # because of the amendments to cms/menu.py, we need to cache for each request_path, so instead of:
+ # key = "%smenu_nodes_%s_%s" % (prefix, lang, site_id)
+ # we do:
+ key = "%smenu_nodes_%s_%s_%s" % (prefix, lang, site_id, hash(request.path))
+
cached_nodes = cache.get(key, None)
if cached_nodes:
return cached_nodes
Something went wrong with that request. Please try again.