[Django] #36674: select_related() leaks memory

[Django]

#36674: select_related() leaks memory
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Type:
| Cleanup/optimization
Status: new | Component: Database
| layer (models, ORM)
Version: dev | Severity: Normal
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Calls to `select_related()` create nested functions that aren't cleaned up
until garbage collection runs:

Borrowed testing strategy from #34865:
{{{#!diff
diff --git a/tests/select_related/tests.py b/tests/select_related/tests.py
index 41ed350cf3..252e884d7a 100644
--- a/tests/select_related/tests.py
+++ b/tests/select_related/tests.py
@@ -242,6 +242,30 @@ class SelectRelatedTests(TestCase):
FETCH_PEERS,
)

+ def test_memory_leak(self):
+ # todo: factor out a test util
+ import gc
+
+ # Schedule the restore of the garbage collection settings.
+ self.addCleanup(gc.set_debug, 0)
+ self.addCleanup(gc.enable)
+
+ # Disable automatic garbage collection to control when it's
triggered,
+ # then run a full collection cycle to ensure `gc.garbage` is
empty.
+ gc.disable()
+ gc.collect()
+
+ # The garbage list isn't automatically populated to avoid CPU
overhead,
+ # so debugging needs to be enabled to track all unreachable items
and
+ # have them stored in `gc.garbage`.
+ gc.set_debug(gc.DEBUG_SAVEALL)
+
+ list(Species.objects.select_related("genus"))
+
+ # Enforce garbage collection to populate `gc.garbage` for
inspection.
+ gc.collect()
+ self.assertEqual(gc.garbage, [])
+

class SelectRelatedValidationTests(SimpleTestCase):
"""
}}}

Gives:
{{{#!diff
AssertionError: Lists differ: [<cell at 0x10630c940: function object at
[148 chars]040>] != []

First list contains 3 additional elements.
First extra element 0:
<cell at 0x10630c940: function object at 0x1079b8040>

+ []
- [<cell at 0x10630c940: function object at 0x1079b8040>,
- (<cell at 0x10630c940: function object at 0x1079b8040>,),
- <function SQLCompiler.get_select.<locals>.get_select_from_parent at
0x1079b8040>]
}}}


At a glance, it looks like we can lift `get_select_from_parent` to a class
method or all the way to module-level without a loss of functionality to
address this.
--
Ticket URL: <https://code.djangoproject.com/ticket/36674>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#36674: select_related() leaks memory
-------------------------------------+-------------------------------------

Reporter: Jacob Walls | Owner: (none)
Type: | Status: new
Cleanup/optimization |
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Simon Charette):

* stage: Unreviewed => Accepted

Comment:

Good catch, looks like it could even be a `staticmethod` since it only
cares about `klass_info`.
--
Ticket URL: <https://code.djangoproject.com/ticket/36674#comment:1>

[Django]

#36674: select_related() leaks memory
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: (none)
Type: | Status: new
Cleanup/optimization |
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by nzioker):

I'm working on this fix.
--
Ticket URL: <https://code.djangoproject.com/ticket/36674#comment:2>

[Django]

#36674: select_related() leaks memory
-------------------------------------+-------------------------------------

Reporter: Jacob Walls | Owner: nzioker
Type: | Status: assigned

Cleanup/optimization |
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by nzioker):

* owner: (none) => nzioker
* status: new => assigned

--
Ticket URL: <https://code.djangoproject.com/ticket/36674#comment:3>

[Django]

#36674: select_related() leaks memory
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: nzioker
Type: | Status: assigned
Cleanup/optimization |
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by nzioker):

* has_patch: 0 => 1

Comment:

PR created: https://github.com/django/django/pull/19977
--
Ticket URL: <https://code.djangoproject.com/ticket/36674#comment:4>

[Django]

#36674: select_related() leaks memory
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: nzioker
Type: | Status: assigned
Cleanup/optimization |
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Simon Charette):

* needs_better_patch: 0 => 1

Comment:

MR was targeting 4.2.

Also it appears that my analysis was wrong. Since we're recursively
calling the function it'd be better if it was a `classmethod` instead so
we can do `cls.get_select_from_parent(ki)` in the function definition. We
should also do `self.get_select_from_parent` instead of
`SQLCompiler.get_select_from_parent` in `.get_select`.
--
Ticket URL: <https://code.djangoproject.com/ticket/36674#comment:5>

[Django]

#36674: select_related() leaks memory
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: nzioker
Type: | Status: assigned
Cleanup/optimization |
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by nzioker):

Here's the new PR: https://github.com/django/django/pull/19986
--
Ticket URL: <https://code.djangoproject.com/ticket/36674#comment:6>