SmileyChris / django-countries

A Django application that provides country choices for use with forms, flag icons static files, and a country field for models.
https://pypi.python.org/pypi/django-countries
MIT License
1.4k stars 289 forks source link

Countries.__iter__ very slow in Django admin #454

Open christianglodt opened 5 months ago

christianglodt commented 5 months ago

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 commented 3 months ago

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 commented 3 months ago

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 listdisplay, 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_forfield 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 commented 3 months ago

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 commented 2 months ago

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)

ronny-rentner commented 4 weeks ago

I have unfortunately the same problem. Just posted the same report in another, 10 year old issue thread as I found that earlier.

Showing a country column in admin makes it 10 times slower than without, so an admin change list view takes 700 ms CPU time instead of 70 ms according to django-debug-toolbar.

I am using django-countries version 7.6.1.

ronny-rentner commented 4 weeks ago

Ok, I think my problem is a different one. @SmileyChris

I've drilled it down to fields.py and the following code in the __init__() method:

        if django.VERSION >= (5, 0):
            # Use new lazy callable support
            kwargs["choices"] = lambda: self.countries
        else:
            kwargs["choices"] = self.countries

The use of that lambda function is causing the slowdown for me. If I comment that block, it's fast and the problem disappears.