Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Meta: 📈 Performance of PageTreeView #886

Closed
4 of 5 tasks
ulliholtgrave opened this issue Jul 7, 2021 · 5 comments
Closed
4 of 5 tasks

Meta: 📈 Performance of PageTreeView #886

ulliholtgrave opened this issue Jul 7, 2021 · 5 comments
Milestone

Comments

@ulliholtgrave
Copy link
Member

ulliholtgrave commented Jul 7, 2021

Motivation

Currently, TreeView loading has some massive perforamce issues when we work with live data. For example, loading the pages of the city of Munich takes almost 2 minutes. This needs to improve significantly before the release.

@ulliholtgrave ulliholtgrave added 💡 feature New feature or request 👼 help wanted Extra attention is needed ❓ question Further information is requested ❗ prio: medium Should be scheduled in the forseeable future. labels Jul 7, 2021
@ulliholtgrave ulliholtgrave added this to the Version 1.0 milestone Jul 7, 2021
@timobrembeck timobrembeck changed the title Improve performance of TreeView Improve performance of PageTreeView Jul 7, 2021
@timobrembeck timobrembeck added 🔨 enhancement This improves an existing feature and removed 💡 feature New feature or request labels Jul 23, 2021
@timobrembeck
Copy link
Member

The same problem probably also impacts the translation report (which takes even longer to load than the page tree)

@svenseeberg svenseeberg added ‼️ prio: high Needs to be resolved ASAP. and removed ❗ prio: medium Should be scheduled in the forseeable future. labels Jul 23, 2021
@svenseeberg
Copy link
Member

svenseeberg commented Sep 14, 2021

The culprit seem to be many small queries that are executed each time the filter() method is being used. We're doing this quite often in model methods, especially those with the @property decorator.

https://docs.djangoproject.com/en/3.2/topics/db/optimization/ provides some insights into how Django internally optimizes database access.

The following things can be done to improve performance:

  • Simply reduce the amount of lookups. Maybe we do not need all info that is being pulled in in some cases?
  • Improve access to required data -> less foreign key lookups.
  • Add Memcached layer.

We should start with the PageTranslation and AbstractBasePageTranslation models, and then continue with Page and AbstractBasePage. Next, we should search filter() calls outside of models, then move them into the models and optimize.

@svenseeberg
Copy link
Member

svenseeberg commented Sep 14, 2021

I enabled the Postgresql query log and collected the queries executed during one API request on the pages endpoint. Each SQL query is one line in the log file.

cat request-filtered.log | wc -l
5612

grep '"cms_page"' request-filtered.log | wc -l
5028

grep '"cms_pagetranslation"' request-filtered.log | wc -l
4299

grep mediafile request-filtered.log | wc -l 
2785

The result contains 68 page translations. That means we execute about 100 SQL queries for each page returned via the API.

@ulliholtgrave
Copy link
Member Author

ulliholtgrave commented Sep 21, 2021

As a kinda dirty approach for reducing the database calls we implemented one example for the abstract_base_page.py. We added an additional value and then changed the get_translation() method to the following:

cached_translations = None
def get_translation(self, language_slug):
translations = self.get_cached_translation()
return next(
filter(
lambda translation: translation.language.slug == language_slug,
list(translations),
),
None,
)
def get_cached_translation(self):
if not self.cached_translations:
self.cached_translations = self.translations.all().select_related(
"language"
)
return self.cached_translations

This removes the filter() function from the property and saves up a lot of database connections.

@svenseeberg svenseeberg changed the title Improve performance of PageTreeView Meta: Improve performance of PageTreeView Sep 28, 2021
@svenseeberg svenseeberg modified the milestones: Version 1.0, Meta Issues Jan 5, 2022
@timobrembeck
Copy link
Member

Since after merging #1149 the performance of the page tree is very good even in regions with many pages, I'm closing this issue now. 🎉

@timobrembeck timobrembeck removed 👼 help wanted Extra attention is needed ❓ question Further information is requested ‼️ prio: high Needs to be resolved ASAP. 🔨 enhancement This improves an existing feature labels Sep 19, 2022
@timobrembeck timobrembeck changed the title Meta: Improve performance of PageTreeView Meta: 📈 Performance of PageTreeView Nov 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants