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

Countries.__iter__ very slow in Django admin #454

Open
christianglodt opened this issue Jan 30, 2024 · 4 comments
Open

Countries.__iter__ very slow in Django admin #454

christianglodt opened this issue Jan 30, 2024 · 4 comments
Labels

Comments

@christianglodt
Copy link

I see a strange performance issue with django-countries 7.5.1. I'm using Django 4.2.8, I have USE_I18N on and I do not have pyuca installed.

In my app there's a django model with a CountryField. Its admin page includes the country field in list_display and list_filter. For some reason, rendering the changelist page takes upward of 6-7 seconds (on a Ryzen 9 5900X CPU). If I remove the country field from list_display and list_filter, the same list renders in around 300ms. This is all with DEBUG on, on the regular development runserver.

I've done a bit of profiling, and found that it seems to be Countries.__iter__ which is slow. I'm not sure how to share the profiling data, so here's a screenshot of a flame graph:

image

Please note that according to the profiler, the top entry for Countries.__iter__ in the flamegraph above corresponds to an execution time of +- 3.4 seconds.

For verification I tried listing the countries in a Python shell (shell_plus from django-extensions) instead, and there the slowdown doesn't happen:

Python 3.11.2 (main, Mar 13 2023, 12:18:29) [GCC 12.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
(InteractiveConsole)
>>> from django_countries import countries
>>> import timeit
>>> timeit.timeit('list(countries)', globals=locals(), number=1)
0.004178638999746909
>>> 

So this indicates to me that there may be a problem having to do with my Django configuration, in particular wrt. translations. But I haven't managed to find out more yet.

I'd be grateful for any help with this issue.

@SmileyChris
Copy link
Owner

SmileyChris commented Apr 1, 2024

Timing the list of countries still wouldn't be triggering the translations of the lazy strings. You'd probably need to time something like [str(c) for c in countries]

@christianglodt
Copy link
Author

Thanks for your reply. I've investigated a bit more, and I've found that the slowness I observe may have to do with the fact that the Countries.__iter__() method is being called many times, and not with a single call being particularly slow.

My testing in the Python interpreter did not reflect that and called it only once, and is thus not realistic.

With the CountryField in list_display, I observe that the Countries.__iter__() method is called twice as many times as the number of instances displayed in the admin changelist view. I've seen this by putting a breakpoint at the start of the __iter__ method, and I got the following stack trace (many times):

__iter__ (/opt/venv/lib/python3.11/site-packages/django_countries/__init__.py:342)
_get_flatchoices (/opt/venv/lib/python3.11/site-packages/django/db/models/fields/__init__.py:1025)
display_for_field (/opt/venv/lib/python3.11/site-packages/django/contrib/admin/utils.py:406)
items_for_result (/opt/venv/lib/python3.11/site-packages/django/contrib/admin/templatetags/admin_list.py:235)
__init__ (/opt/venv/lib/python3.11/site-packages/django/contrib/admin/templatetags/admin_list.py:303)
results (/opt/venv/lib/python3.11/site-packages/django/contrib/admin/templatetags/admin_list.py:312)
result_list (/opt/venv/lib/python3.11/site-packages/django/contrib/admin/templatetags/admin_list.py:336)
render (/opt/venv/lib/python3.11/site-packages/django/template/library.py:258)
render (/opt/venv/lib/python3.11/site-packages/django/contrib/admin/templatetags/base.py:45)
render_annotated (/opt/venv/lib/python3.11/site-packages/django/template/base.py:966)
<listcomp> (/opt/venv/lib/python3.11/site-packages/django/template/base.py:1005)
render (/opt/venv/lib/python3.11/site-packages/django/template/base.py:1005)
render (/opt/venv/lib/python3.11/site-packages/django/template/loader_tags.py:63)
render_annotated (/opt/venv/lib/python3.11/site-packages/django/template/base.py:966)
<listcomp> (/opt/venv/lib/python3.11/site-packages/django/template/base.py:1005)
render (/opt/venv/lib/python3.11/site-packages/django/template/base.py:1005)
render (/opt/venv/lib/python3.11/site-packages/django/template/loader_tags.py:63)
super (/opt/venv/lib/python3.11/site-packages/django/template/loader_tags.py:79)
_resolve_lookup (/opt/venv/lib/python3.11/site-packages/django/template/base.py:914)
resolve (/opt/venv/lib/python3.11/site-packages/django/template/base.py:847)
resolve (/opt/venv/lib/python3.11/site-packages/django/template/base.py:715)
render (/opt/venv/lib/python3.11/site-packages/django/template/base.py:1064)
render_annotated (/opt/venv/lib/python3.11/site-packages/django/template/base.py:966)
<listcomp> (/opt/venv/lib/python3.11/site-packages/django/template/base.py:1005)
render (/opt/venv/lib/python3.11/site-packages/django/template/base.py:1005)
render (/opt/venv/lib/python3.11/site-packages/django/template/loader_tags.py:63)
render_annotated (/opt/venv/lib/python3.11/site-packages/django/template/base.py:966)
<listcomp> (/opt/venv/lib/python3.11/site-packages/django/template/base.py:1005)
render (/opt/venv/lib/python3.11/site-packages/django/template/base.py:1005)
instrumented_test_render (/opt/venv/lib/python3.11/site-packages/django/test/utils.py:112)
render (/opt/venv/lib/python3.11/site-packages/django/template/loader_tags.py:157)
render_annotated (/opt/venv/lib/python3.11/site-packages/django/template/base.py:966)
<listcomp> (/opt/venv/lib/python3.11/site-packages/django/template/base.py:1005)
render (/opt/venv/lib/python3.11/site-packages/django/template/base.py:1005)
instrumented_test_render (/opt/venv/lib/python3.11/site-packages/django/test/utils.py:112)
render (/opt/venv/lib/python3.11/site-packages/django/template/loader_tags.py:157)
render_annotated (/opt/venv/lib/python3.11/site-packages/django/template/base.py:966)
<listcomp> (/opt/venv/lib/python3.11/site-packages/django/template/base.py:1005)
render (/opt/venv/lib/python3.11/site-packages/django/template/base.py:1005)

It seems Django calls display_for_field for every instance in the changelist, which calls _get_flatchoices, which iterates over all choices provided by Countries.__iter__(). The choices are translated and sorted every time in Countries.__iter__().

It would be great if it would be possible to cache the result.

@christianglodt
Copy link
Author

Just as an additional data point, if I make a crude custom CountryField like this:

class CachingCountryField(CountryField):
    def __init__(self, *args, **kwargs):
        super().__init__(self, *args, **kwargs)
        self.choices = list(self.choices)

With this, the page rendering time for the changelist view in my development server goes down from +-10.000 ms to +-450ms.

Obviously this is not a real solution. But I think it shows that caching would be worthwhile.

@SmileyChris
Copy link
Owner

Thanks for your findings! I'll take another dive into why this is happening soon. Caching would be a potential way to bandaid fix this and probably a good efficiency improvement anyway, however the caching does need to take into account the current active translation and not just hard code the list (hence why it's being done lazily currently)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants