search

django-cms / djangocms-versioning

General purpose versioning package for Django CMS 4 and above.
Other
33 stars 29 forks source link

Speed/performance issue with the menu #379

Closed marksweb closed 3 months ago

marksweb commented 4 months ago

Following an issue raised on discord and posted here; https://github.com/django-cms/django-cms/discussions/7805

It has been established that the issue likely rests with the menu of versioning, because the issue was not present in django-cms 3.

There looks to be a prefetch_related that could be added for the PageURL table which gets called when the menus get the absolute URL of a page.

marksweb commented 4 months ago

Using a count on the queries through tests should mean we can add the prefetch_related and ensure that it doesn't regress in future.

django.test.utils.CaptureQueriesContext()

Some example usage can be found here; https://www.programcreek.com/python/example/74788/django.test.utils.CaptureQueriesContext

fsbraun commented 4 months ago

Maybe it's worth looking at this queryset which does not prefetch the PageURL: https://github.com/django-cms/djangocms-versioning/blob/8f5c0aef531dec37d191ff2642e95db46815fa53/djangocms_versioning/cms_menus.py#L105-L112 On first sight, a select related for page__urls is missing.

fsbraun commented 4 months ago

I wonder if at this point it makes sense to "fix" the page menus in the core using the admin_manager and its current_content filter and remove versioning's re-implementation.

The key difference between those two implementations is that the core works on pages and versioning on page contents. Whichever is more performant, imho should be used in one place: the core. (My guess: versionings implementation.)

fsbraun commented 3 months ago

Fixed with #388