Improve performance of page tree #1088
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
timobrembeck
force-pushed
the
refactoring/page-performance
branch
from
6f79a65
to
21fb977
Compare
svenseeberg
reviewed
Jan 5, 2022
There was a problem hiding this comment.
In template integreat_cms/cms/templates/pages/page_tree_node.html, error at line 118
throws a 'NoneType' object has no attribute 'slug'
Full trace:
Jan 05 20:21:18 ERROR django.request - Internal Server Error: /nurnberg/pages/de/
Traceback (most recent call last):
File "integreat-cms-django/.venv/lib64/python3.9/site-packages/django/core/handlers/exception.py", line 47, in inner
response = get_response(request)
File "integreat-cms-django/.venv/lib64/python3.9/site-packages/django/core/handlers/base.py", line 181, in _get_response
response = wrapped_callback(request, *callback_args, **callback_kwargs)
File "integreat-cms-django/.venv/lib64/python3.9/site-packages/django/views/generic/base.py", line 70, in view
return self.dispatch(request, *args, **kwargs)
File "integreat-cms-django/.venv/lib64/python3.9/site-packages/django/utils/decorators.py", line 43, in _wrapper
return bound_method(*args, **kwargs)
File "integreat-cms-django/.venv/lib64/python3.9/site-packages/django/contrib/auth/decorators.py", line 21, in _wrapped_view
return view_func(request, *args, **kwargs)
File "integreat-cms-django/.venv/lib64/python3.9/site-packages/django/utils/decorators.py", line 43, in _wrapper
return bound_method(*args, **kwargs)
File "integreat-cms-django/integreat_cms/cms/decorators.py", line 114, in wrap
return function(request, *args, **kwargs)
File "integreat-cms-django/.venv/lib64/python3.9/site-packages/django/utils/decorators.py", line 43, in _wrapper
return bound_method(*args, **kwargs)
File "integreat-cms-django/.venv/lib64/python3.9/site-packages/django/contrib/auth/decorators.py", line 21, in _wrapped_view
return view_func(request, *args, **kwargs)
File "integreat-cms-django/.venv/lib64/python3.9/site-packages/django/views/generic/base.py", line 98, in dispatch
return handler(request, *args, **kwargs)
File "integreat-cms-django/integreat_cms/cms/views/pages/page_tree_view.py", line 148, in get
return render(
File "integreat-cms-django/.venv/lib64/python3.9/site-packages/django/shortcuts.py", line 19, in render
content = loader.render_to_string(template_name, context, request, using=using)
File "integreat-cms-django/.venv/lib64/python3.9/site-packages/django/template/loader.py", line 62, in render_to_string
return template.render(context, request)
File "integreat-cms-django/.venv/lib64/python3.9/site-packages/django/template/backends/django.py", line 61, in render
return self.template.render(context)
File "integreat-cms-django/.venv/lib64/python3.9/site-packages/django/template/base.py", line 170, in render
return self._render(context)
File "integreat-cms-django/.venv/lib64/python3.9/site-packages/django/test/utils.py", line 100, in instrumented_test_render
return self.nodelist.render(context)
File "integreat-cms-django/.venv/lib64/python3.9/site-packages/django/template/base.py", line 938, in render
bit = node.render_annotated(context)
File "integreat-cms-django/.venv/lib64/python3.9/site-packages/django/template/base.py", line 905, in render_annotated
return self.render(context)
File "integreat-cms-django/.venv/lib64/python3.9/site-packages/django/template/loader_tags.py", line 150, in render
return compiled_parent._render(context)
File "integreat-cms-django/.venv/lib64/python3.9/site-packages/django/test/utils.py", line 100, in instrumented_test_render
return self.nodelist.render(context)
File "integreat-cms-django/.venv/lib64/python3.9/site-packages/django/template/base.py", line 938, in render
bit = node.render_annotated(context)
File "integreat-cms-django/.venv/lib64/python3.9/site-packages/django/template/base.py", line 905, in render_annotated
return self.render(context)
File "integreat-cms-django/.venv/lib64/python3.9/site-packages/django/template/loader_tags.py", line 150, in render
return compiled_parent._render(context)
File "integreat-cms-django/.venv/lib64/python3.9/site-packages/django/test/utils.py", line 100, in instrumented_test_render
return self.nodelist.render(context)
File "integreat-cms-django/.venv/lib64/python3.9/site-packages/django/template/base.py", line 938, in render
bit = node.render_annotated(context)
File "integreat-cms-django/.venv/lib64/python3.9/site-packages/django/template/base.py", line 905, in render_annotated
return self.render(context)
File "integreat-cms-django/.venv/lib64/python3.9/site-packages/django/template/loader_tags.py", line 62, in render
result = block.nodelist.render(context)
File "integreat-cms-django/.venv/lib64/python3.9/site-packages/django/template/base.py", line 938, in render
bit = node.render_annotated(context)
File "integreat-cms-django/.venv/lib64/python3.9/site-packages/django/template/base.py", line 905, in render_annotated
return self.render(context)
File "integreat-cms-django/.venv/lib64/python3.9/site-packages/django/template/loader_tags.py", line 62, in render
result = block.nodelist.render(context)
File "integreat-cms-django/.venv/lib64/python3.9/site-packages/django/template/base.py", line 938, in render
bit = node.render_annotated(context)
File "integreat-cms-django/.venv/lib64/python3.9/site-packages/django/template/base.py", line 905, in render_annotated
return self.render(context)
File "integreat-cms-django/.venv/lib64/python3.9/site-packages/django/template/defaulttags.py", line 312, in render
return nodelist.render(context)
File "integreat-cms-django/.venv/lib64/python3.9/site-packages/django/template/base.py", line 938, in render
bit = node.render_annotated(context)
File "integreat-cms-django/.venv/lib64/python3.9/site-packages/django/template/base.py", line 905, in render_annotated
return self.render(context)
File "integreat-cms-django/.venv/lib64/python3.9/site-packages/django_mptt-0.13.4-py3.9.egg/mptt/templatetags/mptt_tags.py", line 313, in render
bits = [self._render_node(context, node) for node in roots]
File "integreat-cms-django/.venv/lib64/python3.9/site-packages/django_mptt-0.13.4-py3.9.egg/mptt/templatetags/mptt_tags.py", line 313, in <listcomp>
bits = [self._render_node(context, node) for node in roots]
File "integreat-cms-django/.venv/lib64/python3.9/site-packages/django_mptt-0.13.4-py3.9.egg/mptt/templatetags/mptt_tags.py", line 303, in _render_node
bits.append(self._render_node(context, child))
File "integreat-cms-django/.venv/lib64/python3.9/site-packages/django_mptt-0.13.4-py3.9.egg/mptt/templatetags/mptt_tags.py", line 306, in _render_node
rendered = self.template_nodes.render(context)
File "integreat-cms-django/.venv/lib64/python3.9/site-packages/django/template/base.py", line 938, in render
bit = node.render_annotated(context)
File "integreat-cms-django/.venv/lib64/python3.9/site-packages/django/template/base.py", line 905, in render_annotated
return self.render(context)
File "integreat-cms-django/.venv/lib64/python3.9/site-packages/django/template/loader_tags.py", line 195, in render
return template.render(context)
File "integreat-cms-django/.venv/lib64/python3.9/site-packages/django/template/base.py", line 172, in render
return self._render(context)
File "integreat-cms-django/.venv/lib64/python3.9/site-packages/django/test/utils.py", line 100, in instrumented_test_render
return self.nodelist.render(context)
File "integreat-cms-django/.venv/lib64/python3.9/site-packages/django/template/base.py", line 938, in render
bit = node.render_annotated(context)
File "integreat-cms-django/.venv/lib64/python3.9/site-packages/django/template/base.py", line 905, in render_annotated
return self.render(context)
File "integreat-cms-django/.venv/lib64/python3.9/site-packages/django/template/defaulttags.py", line 312, in render
return nodelist.render(context)
File "integreat-cms-django/.venv/lib64/python3.9/site-packages/django/template/base.py", line 938, in render
bit = node.render_annotated(context)
File "integreat-cms-django/.venv/lib64/python3.9/site-packages/django/template/base.py", line 905, in render_annotated
return self.render(context)
File "integreat-cms-django/.venv/lib64/python3.9/site-packages/django/template/base.py", line 988, in render
output = self.filter_expression.resolve(context)
File "integreat-cms-django/.venv/lib64/python3.9/site-packages/django/template/base.py", line 671, in resolve
obj = self.var.resolve(context)
File "integreat-cms-django/.venv/lib64/python3.9/site-packages/django/template/base.py", line 796, in resolve
value = self._resolve_lookup(context)
File "integreat-cms-django/.venv/lib64/python3.9/site-packages/django/template/base.py", line 858, in _resolve_lookup
current = current()
File "integreat-cms-django/integreat_cms/cms/models/pages/abstract_base_page_translation.py", line 112, in get_absolute_url
return "/" + self.permalink
File "integreat-cms-django/integreat_cms/cms/models/pages/page_translation.py", line 100, in permalink
self.ancestor_path,
File "integreat-cms-django/integreat_cms/cms/models/pages/page_translation.py", line 78, in ancestor_path
[
File "integreat-cms-django/integreat_cms/cms/models/pages/page_translation.py", line 81, in <listcomp>
else ancestor.default_translation.slug
AttributeError: 'NoneType' object has no attribute 'slug'
fa07116
to
cc5a3dc
Compare
timobrembeck
changed the title
timobrembeck
force-pushed
the
refactoring/page-performance
branch
from
cc5a3dc
to
00d556f
Compare
|
Well, this rabbit hole was deeper than I anticipated, and I'm still not completely happy with the current solution, but I think it's ready to be tested now. As usual, I'm sorry about the huge diff 😒 |
timobrembeck
force-pushed
the
refactoring/page-performance
branch
from
00d556f
to
442a078
Compare
timobrembeck
force-pushed
the
refactoring/page-performance
branch
2 times, most recently
from
583f746
to
cde167a
Compare
|
I did a little bit of benchmarking of the loading times in milliseconds on my local machine (so no valid absolute numbers, but maybe interesting for a qualitative comparison): API pages endpoint:
Page TreeView:
You can reproduce similar results by using the new |
|
The numbers sound very promising :) |
|
Does it make sense to only do a rough review of this PR and then merge it to develop and send all devs bug hunting on our test system? |
Probably, yes... there will definitely be a few bugs hidden in these lines, but I don't expect anyone to do a full code review on this... |
timobrembeck
force-pushed
the
refactoring/page-performance
branch
from
cde167a
to
9fec97f
Compare
|
Speaking of which, I found another small bug in the imprint form, but should be fixed now. Maybe it's a good time to write more automated tests to check form submissions etc... |
melegiul
suggested changes
Jan 17, 2022
There was a problem hiding this comment.
Thank you for this great job, loading of the page tree with the new test data looks much better now
I tested the drag and drop feature at the page tree and the language tree and I found an exception (IndexError: list index out of range - POST /augsburg/pages/de/23/move/22/last-child) after some first valid moves, which might be related to the new treebeard package
Traceback
Traceback (most recent call last):
File "/home/melegiul/dev/integreat-cms/.venv/lib/python3.9/site-packages/django/core/handlers/exception.py", line 47, in inner
response = get_response(request)
File "/home/melegiul/dev/integreat-cms/.venv/lib/python3.9/site-packages/django/core/handlers/base.py", line 181, in _get_response
response = wrapped_callback(request, *callback_args, **callback_kwargs)
File "/home/melegiul/dev/integreat-cms/.venv/lib/python3.9/site-packages/django/views/decorators/http.py", line 40, in inner
return func(request, *args, **kwargs)
File "/home/melegiul/dev/integreat-cms/.venv/lib/python3.9/site-packages/django/contrib/auth/decorators.py", line 21, in _wrapped_view
return view_func(request, *args, **kwargs)
File "/home/melegiul/dev/integreat-cms/integreat_cms/cms/decorators.py", line 112, in wrap
return function(request, *args, **kwargs)
File "/home/melegiul/dev/integreat-cms/.venv/lib/python3.9/site-packages/django/contrib/auth/decorators.py", line 21, in _wrapped_view
return view_func(request, *args, **kwargs)
File "/home/melegiul/dev/integreat-cms/integreat_cms/cms/views/pages/page_actions.py", line 547, in move_page
page.save()
File "/home/melegiul/dev/integreat-cms/integreat_cms/cms/models/abstract_tree_node.py", line 106, in save
self.parent = self.get_parent()
File "/home/melegiul/dev/integreat-cms/.venv/lib/python3.9/site-packages/treebeard/ns_tree.py", line 661, in get_parent
self._cached_parent_obj = self.get_ancestors().reverse()[0]
File "/home/melegiul/dev/integreat-cms/.venv/lib/python3.9/site-packages/django/db/models/query.py", line 318, in __getitem__
return qs._result_cache[0]
9fec97f
to
f8cb601
Compare
@melegiul Thanks a lot for the report, I could reproduce the issue locally and hopefully fix it. Could you test again? |
melegiul
suggested changes
Jan 18, 2022
timobrembeck
force-pushed
the
refactoring/page-performance
branch
from
f8cb601
to
b9196ef
Compare
melegiul
approved these changes
Jan 18, 2022
timobrembeck
force-pushed
the
refactoring/page-performance
branch
2 times, most recently
from
6bf7649
to
1d7ee93
Compare
For two reasons:
1. django-mptt is unmaintained
2. django-mptt doesn't allow prefetching translations in page tree
- Prefetch page translations in content lists and API - Prefetch language tree together with region - Use cached_property in content models
Pass the get-parameter "debug" to any JSON view to get the result in HTML including the debug toolbar
- Add current region to request object in custom middleware - Remove context processor which added the region to the template context This prevents duplicated region queries in views and templates
- Add initial migration file - Unignore migrations directory - Add CircleCI check whether migrations are missing
- Add more pages to the test data fixture - Add dev tool to generate more test data locally - Reflect changes in the expected outputs of the API tests
timobrembeck
force-pushed
the
refactoring/page-performance
branch
from
1d7ee93
to
b38b114
Compare
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Short description
Improve performance of page tree
Proposed changes
Resolved issues
This is part of #886 and #943 and leads to various performance improvements
Fixes: #642
Fixes: #748