django-auto-prefetch icon indicating copy to clipboard operation
django-auto-prefetch copied to clipboard

Don't create the weak dicts for individual objects

Open tolomea opened this issue 4 years ago • 4 comments

I have a memory profile with > 60k each of weakref, KeyedRef and WeakValueDictionary, this is the most likely source. Also I think the improvement is good regardless. There is already handling in DescriptorMixin._should_prefetch for _peers being unset, that code also does the same >= 2 check.

tolomea avatar Aug 24 '21 15:08 tolomea

obvs that suggests I have 60,000 model instances floating around which I do, bu that's going to be a harder fix

tolomea avatar Aug 24 '21 15:08 tolomea

Smart 🧠. You could drop the >= 2 check in should_prefetch and just have a check that the _peers attr exists, to avoid repeating the magic number 2.

Also add a HISTORY.rst note before releasing?

adamchainz avatar Aug 24 '21 15:08 adamchainz

Because of the weakdict nature it can drop below 2 after loading, so I think the other check should stay.

tolomea avatar Aug 24 '21 15:08 tolomea

Good point

adamchainz avatar Aug 24 '21 15:08 adamchainz

When will this be merged. Can I help you merge it somehow? I ran across same weakdict-based stuff and trying to resolve it

I cannot cache objects because of weakdicts:

from django.core.cache import caches
from tv.apps.core import models
p = models.Packet.objects.get(pk=113)
caches['default'].set('asddsa', p, 60)

Traceback (most recent call last):
  File "/home/dk/.pyenv/versions/p38/lib/python3.8/site-packages/IPython/core/interactiveshell.py", line 3417, in run_code
    exec(code_obj, self.user_global_ns, self.user_ns)
  File "<ipython-input-1-7969eb7b9b95>", line 4, in <module>
    caches['default'].set('asddsa', p, 60)
  File "/home/dk/.pyenv/versions/p38/lib/python3.8/site-packages/django_redis/cache.py", line 27, in _decorator
    return method(self, *args, **kwargs)
  File "/home/dk/.pyenv/versions/p38/lib/python3.8/site-packages/django_redis/cache.py", line 76, in set
    return self.client.set(*args, **kwargs)
  File "/home/dk/.pyenv/versions/p38/lib/python3.8/site-packages/django_redis/client/default.py", line 126, in set
    nvalue = self.encode(value)
  File "/home/dk/.pyenv/versions/p38/lib/python3.8/site-packages/django_redis/client/default.py", line 358, in encode
    value = self._serializer.dumps(value)
  File "/home/dk/.pyenv/versions/p38/lib/python3.8/site-packages/django_redis/serializers/pickle.py", line 21, in dumps
    return pickle.dumps(value, self._pickle_version)
AttributeError: Can't pickle local object 'WeakValueDictionary.__init__.<locals>.remove'

dkgitdev avatar Nov 09 '22 16:11 dkgitdev

I didn't realize this hadn't been merged, @adamchainz could you sort out the HISTORY.rst conflict?

@dkgitdev That shouldn't be happening and this won't fix it. It suggests that either you have a getstate function that isn't calling super. Or you have a queryset that is inheriting off auto_prefetch.QuerySet, but it's model is not inheriting off auto_prefetch.Model.

tolomea avatar Nov 09 '22 17:11 tolomea

@dkgitdev That shouldn't be happening and this won't fix it. It suggests that either you have a getstate function that isn't calling super. Or you have a queryset that is inheriting off auto_prefetch.QuerySet, but it's model is not inheriting off auto_prefetch.Model.

Thank you, this specific model was not using auto_prefetch.Model

UPD: I've created a feature request for quality of life feature with the file checks: #197

dkgitdev avatar Nov 09 '22 19:11 dkgitdev