8000 chore: Improve pagecontent caching in page admin (esp. page tree) by fsbraun · Pull Request #8002 · django-cms/django-cms · GitHub
[go: up one dir, main page]

Skip to content

chore: Improve pagecontent caching in page admin (esp. page tree) #8002

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

Merged
merged 5 commits into from
Sep 19, 2024

Conversation

fsbraun
Copy link
Member
@fsbraun fsbraun commented Sep 18, 2024

Description

Historically, django CMS has used a python cache on model level for page content objects: page.page_content_cache.

Since django CMS 4., this cache is filled either with user-visible objects (using the default objects manager) or with admin-visible objects, using the admin_manager manager. Since each request is, either from the user side or the admin side, this same cache can be used to hold different sets of page content objects. A side effect, however, lead to some page titles in fallback languages not to be shown in the page tree.

It turns out, however, that this was not consistently done, leading to unnecessary cache misses.

This PR splits the caches into page_content_cache(traditional, user-facing) and admin_content_cache for the page admin, to make sure no side effects can happen.

This improves the cache hits and reduces the number of database hits for the page tree effectively removing a hidden N+1 issue. The response time should improve by 20%+ (tested with local sqlite).

Related resources

  • #...
  • #...

Checklist

  • I have opened this pull request against develop-4
  • I have added or modified the tests when changing logic
  • I have followed the conventional commits guidelines to add meaningful information into the changelog
  • I have read the contribution guidelines and I have joined the channel #pr-reviews on our Discord Server to find a “pr review buddy” who is going to review my pull request.

@fsbraun fsbraun requested a review from a team September 18, 2024 09:50
@fsbraun fsbraun added the needs to be backported Commits need to be backported label Sep 18, 2024
@fsbraun fsbraun merged commit 921c020 into django-cms:develop-4 Sep 19, 2024
32 checks passed
fsbraun added a commit to fsbraun/django-cms that referenced this pull request Sep 19, 2024
…ango-cms#8002)

* Fix: Separate cache and access methods for page admin to hold available page contents

* Fix typo

* Remove `EmptyPageContent` from admin_content_cache

* Optimize queryset for pagetree

---------

Co-authored-by: Github Release Action <info@django-cms.org>
fsbraun added a commit that referenced this pull request Sep 20, 2024
…e tree) (#8002) (#8004)

* chore: Improve pagecontent caching in page admin (esp. page tree) (#8002)

* Fix: Separate cache and access methods for page admin to hold available page contents

* Fix typo

* Remove `EmptyPageContent` from admin_content_cache

* Optimize queryset for pagetree

---------

Co-authored-by: Github Release Action <info@django-cms.org>

* Add back template file

* Undo permission check change (error when merging)

* Undo more merge errors

* Fix: missing warning import

* Clarify an isort/ruff dispute

* Add write permission

* Add admin cache to cms_toolbars.py

* fix typo in cms_toolbars.py

---------

Co-authored-by: Github Release Action <info@django-cms.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4.1 5.0 needs to be backported Commits need to be backported
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants
0