Test for short term reference cycles and resolve as many as possible

Added test support and repaired a wide variety of unnecessary reference
cycles created for short-lived objects, mostly in the area of ORM queries.

Fixes: #5056
Change-Id: Ifd93856eba550483f95f9ae63d49f36ab068b85a

6 files changed, 19 insertions, 6 deletions

diff --git a/lib/sqlalchemy/sql/annotation.py b/lib/sqlalchemy/sql/annotation.py
index 9853cef2a..447bbe667 100644
--- a/lib/sqlalchemy/sql/annotation.py
+++ b/lib/sqlalchemy/sql/annotation.py
@@ -247,6 +247,7 @@ def _deep_annotate(element, annotations, exclude=None):
 
     if element is not None:
         element = clone(element)
+    clone = None  # remove gc cycles
     return element
 
 
@@ -271,6 +272,7 @@ def _deep_deannotate(element, values=None):
 
     if element is not None:
         element = clone(element)
+    clone = None  # remove gc cycles
     return element
 
 
diff --git a/lib/sqlalchemy/sql/elements.py b/lib/sqlalchemy/sql/elements.py
index 7d857d4fe..c16c2f0ca 100644
--- a/lib/sqlalchemy/sql/elements.py
+++ b/lib/sqlalchemy/sql/elements.py
@@ -255,6 +255,12 @@ class ClauseElement(
         """
         s = util.column_set()
         f = self
+
+        # note this creates a cycle, asserted in test_memusage. however,
+        # turning this into a plain @property adds tends of thousands of method
+        # calls to Core / ORM performance tests, so the small overhead
+        # introduced by the relatively small amount of short term cycles
+        # produced here is preferable
         while f is not None:
             s.add(f)
             f = f._is_clone_of
diff --git a/lib/sqlalchemy/sql/selectable.py b/lib/sqlalchemy/sql/selectable.py
index ed7a6c2b9..428acae6c 100644
--- a/lib/sqlalchemy/sql/selectable.py
+++ b/lib/sqlalchemy/sql/selectable.py
@@ -3695,10 +3695,10 @@ class Select(
             clone(f, **kw) for f in self._from_obj
         ).union(f for f in new_froms.values() if isinstance(f, Join))
 
-        self._correlate = set(clone(f) for f in self._correlate)
+        self._correlate = set(clone(f, **kw) for f in self._correlate)
         if self._correlate_except:
             self._correlate_except = set(
-                clone(f) for f in self._correlate_except
+                clone(f, **kw) for f in self._correlate_except
             )
 
         # 4. clone other things.   The difficulty here is that Column
diff --git a/lib/sqlalchemy/sql/traversals.py b/lib/sqlalchemy/sql/traversals.py
index 84a5623d3..68a3a0749 100644
--- a/lib/sqlalchemy/sql/traversals.py
+++ b/lib/sqlalchemy/sql/traversals.py
@@ -29,6 +29,8 @@ def compare(obj1, obj2, **kw):
 class HasCacheKey(object):
     _cache_key_traversal = NO_CACHE
 
+    __slots__ = ()
+
     def _gen_cache_key(self, anon_map, bindparams):
         """return an optional cache key.
 
diff --git a/lib/sqlalchemy/sql/util.py b/lib/sqlalchemy/sql/util.py
index 8539f4845..546d989eb 100644
--- a/lib/sqlalchemy/sql/util.py
+++ b/lib/sqlalchemy/sql/util.py
@@ -226,6 +226,7 @@ def visit_binary_product(fn, expr):
                     yield e
 
     list(visit(expr))
+    visit = None  # remove gc cycles
 
 
 def find_tables(
@@ -881,7 +882,7 @@ class ColumnAdapter(ClauseAdapter):
             anonymize_labels=anonymize_labels,
         )
 
-        self.columns = util.populate_column_dict(self._locate_col)
+        self.columns = util.WeakPopulateDict(self._locate_col)
         if self.include_fn or self.exclude_fn:
             self.columns = self._IncludeExcludeMapping(self, self.columns)
         self.adapt_required = adapt_required
@@ -907,7 +908,7 @@ class ColumnAdapter(ClauseAdapter):
         ac = self.__class__.__new__(self.__class__)
         ac.__dict__.update(self.__dict__)
         ac._wrap = adapter
-        ac.columns = util.populate_column_dict(ac._locate_col)
+        ac.columns = util.WeakPopulateDict(ac._locate_col)
         if ac.include_fn or ac.exclude_fn:
             ac.columns = self._IncludeExcludeMapping(ac, ac.columns)
 
@@ -942,4 +943,4 @@ class ColumnAdapter(ClauseAdapter):
 
     def __setstate__(self, state):
         self.__dict__.update(state)
-        self.columns = util.PopulateDict(self._locate_col)
+        self.columns = util.WeakPopulateDict(self._locate_col)
diff --git a/lib/sqlalchemy/sql/visitors.py b/lib/sqlalchemy/sql/visitors.py
index dcded3484..afa1506d2 100644
--- a/lib/sqlalchemy/sql/visitors.py
+++ b/lib/sqlalchemy/sql/visitors.py
@@ -722,7 +722,7 @@ def cloned_traverse(obj, opts, visitors):
                         return newelem
 
                 cloned[id(elem)] = newelem = elem._clone()
-                newelem._copy_internals(clone=clone)
+                newelem._copy_internals(clone=clone, **kw)
                 meth = visitors.get(newelem.__visit_name__, None)
                 if meth:
                     meth(newelem)
@@ -730,6 +730,7 @@ def cloned_traverse(obj, opts, visitors):
 
     if obj is not None:
         obj = clone(obj)
+    clone = None  # remove gc cycles
     return obj
 
 
@@ -786,4 +787,5 @@ def replacement_traverse(obj, opts, replace):
 
     if obj is not None:
         obj = clone(obj, **opts)
+    clone = None  # remove gc cycles
     return obj