Permalink
Browse files

simple way to speed up menus

  • Loading branch information...
1 parent 81045d9 commit 17564d501487b8f3adf32dc8b0c260b28e25fba7 @evildmp committed Jun 12, 2012
Showing with 58 additions and 0 deletions.
  1. +52 −0 cms/menu.py
  2. +6 −0 menus/menu_pool.py
View
52 cms/menu.py
@@ -234,6 +234,58 @@ 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
+
+ # 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

4 comments on commit 17564d5

@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.

@FrankBie

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

@evildmp
Owner
@FrankBie

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

Please sign in to comment.