remove intermediary _is_clone_of entries when cloning

Improvements in memory usage by the ORM, removing a significant set of
intermediary expression objects that are typically stored when a copy of an
expression object is created. These clones have been greatly reduced,
reducing the number of total expression objects stored in memory by
ORM mappings by about 30%.

note this change causes the tests to have a bit of a harder time with
GC, which we would assume is because mappings now have a lot more
garbage to clean up after mappers are configured.   it remains
to be seen what the long term effects of this are.

Fixes: #7823
Change-Id: If8729747ffb9bf27e8974f069a994b5a823ee095

diff --git a/doc/build/changelog/unreleased_14/7823.rst b/doc/build/changelog/unreleased_14/7823.rst
new file mode 100644 (file)
index 0000000..249a749
--- /dev/null
+++ b/doc/build/changelog/unreleased_14/7823.rst
@@ -0,0 +1,9 @@
+.. change::
+    :tags: bug, orm, performance
+    :tickets: 7823
+
+    Improvements in memory usage by the ORM, removing a significant set of
+    intermediary expression objects that are typically stored when a copy of an
+    expression object is created. These clones have been greatly reduced,
+    reducing the number of total expression objects stored in memory by
+    ORM mappings by about 30%.

diff --git a/lib/sqlalchemy/sql/elements.py b/lib/sqlalchemy/sql/elements.py
index fdb3fc8bbf892542ae388a887a86ea1745b7457b..346a84ddfa6d9223d965898cc8c681bbe66ced0d 100644 (file)
--- a/lib/sqlalchemy/sql/elements.py
+++ b/lib/sqlalchemy/sql/elements.py
@@ -357,7 +357,8 @@ class ClauseElement(
         # process leaves around a lot of remnants of the previous clause
         # typically in the form of column expressions still attached to the
         # old table.
-        c._is_clone_of = self
+        cc = self._is_clone_of
+        c._is_clone_of = cc if cc is not None else self
 
         return c
 

diff --git a/lib/sqlalchemy/sql/selectable.py b/lib/sqlalchemy/sql/selectable.py
index 0692483e9f2df48de8e03eeef74f86e1b47f49c7..e143d1476182318b94065b463815b1a1e137e2b4 100644 (file)
--- a/lib/sqlalchemy/sql/selectable.py
+++ b/lib/sqlalchemy/sql/selectable.py
@@ -4135,7 +4135,8 @@ class _MemoizedSelectEntities(
     def _clone(self, **kw):
         c = self.__class__.__new__(self.__class__)
         c.__dict__ = {k: v for k, v in self.__dict__.items()}
-        c._is_clone_of = self
+
+        c._is_clone_of = self.__dict__.get("_is_clone_of", self)
         return c
 
     @classmethod

diff --git a/test/aaa_profiling/test_memusage.py b/test/aaa_profiling/test_memusage.py
index cf89dd6e2b8e9bcd4e4f13ad36f0ff5cfe37678f..f084eac2c0543ba2c6dd0b6b5bc6b1f7e4835e93 100644 (file)
--- a/test/aaa_profiling/test_memusage.py
+++ b/test/aaa_profiling/test_memusage.py
@@ -346,6 +346,19 @@ class MemUsageTest(EnsureZeroed):
 
         go()
 
+    def test_clone_expression(self):
+
+        root_expr = column("x", Integer) == 12
+        expr = root_expr
+
+        @profile_memory()
+        def go():
+            nonlocal expr
+
+            expr = cloned_traverse(expr, {}, {})
+
+        go()
+
 
 @testing.add_to_marker.memory_intensive
 class MemUsageWBackendTest(fixtures.MappedTest, EnsureZeroed):

diff --git a/test/orm/test_hasparent.py b/test/orm/test_hasparent.py
index 425dd947d4cef9e3fb42610b42e1400ea98b5696..8f61c11970db713ea49d1b968ffd53f2a324d319 100644 (file)
--- a/test/orm/test_hasparent.py
+++ b/test/orm/test_hasparent.py
@@ -1,5 +1,4 @@
 """test the current state of the hasparent() flag."""
-
 from sqlalchemy import ForeignKey
 from sqlalchemy import Integer
 from sqlalchemy import testing
@@ -26,6 +25,10 @@ class ParentRemovalTest(fixtures.MappedTest):
 
     run_inserts = None
 
+    # trying to push GC to do a better job
+    run_setup_classes = "each"
+    run_setup_mappers = "each"
+
     @classmethod
     def define_tables(cls, metadata):
         if testing.against("oracle"):
@@ -173,12 +176,24 @@ class ParentRemovalTest(fixtures.MappedTest):
         """
         User = self.classes.User
         s, u1, a1 = self._fixture()
+        gc_collect()
 
         u2 = User(addresses=[a1])  # noqa
 
         s.expire(a1)
         u1.addresses.remove(a1)
 
+        u2_is = u2._sa_instance_state
+        del u2
+
+        for i in range(5):
+            gc_collect()
+        # heisenberg the GC a little bit, since #7823 caused a lot more
+        # GC when mappings are set up, larger test suite started failing
+        # on this being gc'ed
+        o = u2_is.obj()
+        assert o is None
+
         # controversy here.  The action is
         # to expire one object, not the other, and remove;
         # this is pretty abusive in any case. for now
@@ -192,13 +207,23 @@ class ParentRemovalTest(fixtures.MappedTest):
     def test_stale_state_negative(self):
         User = self.classes.User
         s, u1, a1 = self._fixture()
+        gc_collect()
 
         u2 = User(addresses=[a1])
         s.add(u2)
         s.flush()
         s._expunge_states([attributes.instance_state(u2)])
+
+        u2_is = u2._sa_instance_state
         del u2
-        gc_collect()
+
+        for i in range(5):
+            gc_collect()
+        # heisenberg the GC a little bit, since #7823 caused a lot more
+        # GC when mappings are set up, larger test suite started failing
+        # on this being gc'ed
+        o = u2_is.obj()
+        assert o is None
 
         assert_raises_message(
             orm_exc.StaleDataError,

diff --git a/test/orm/test_session.py b/test/orm/test_session.py
index d146612a5a159e10426a0cd9ff9d1939891c81c5..56371bc68c7b534135db8d6b643aa18c79ec4061 100644 (file)
--- a/test/orm/test_session.py
+++ b/test/orm/test_session.py
@@ -1568,14 +1568,22 @@ class WeakIdentityMapTest(_fixtures.FixtureTest):
 
         s = fixture_session()
         self.mapper_registry.map_imperatively(User, users)
+        gc_collect()
 
         s.add(User(name="ed"))
         s.flush()
         assert not s.dirty
 
         user = s.query(User).one()
+
+        # heisenberg the GC a little bit, since #7823 caused a lot more
+        # GC when mappings are set up, larger test suite started failing
+        # on this being gc'ed
+        user_is = user._sa_instance_state
         del user
         gc_collect()
+        assert user_is.obj() is None
+
         assert len(s.identity_map) == 0
 
         user = s.query(User).one()
@@ -1600,6 +1608,7 @@ class WeakIdentityMapTest(_fixtures.FixtureTest):
 
         s = fixture_session()
         self.mapper_registry.map_imperatively(User, users)
+        gc_collect()
 
         s.add(User(name="ed"))
         s.flush()
@@ -1642,6 +1651,8 @@ class WeakIdentityMapTest(_fixtures.FixtureTest):
             properties={"addresses": relationship(Address, backref="user")},
         )
         self.mapper_registry.map_imperatively(Address, addresses)
+        gc_collect()
+
         s.add(User(name="ed", addresses=[Address(email_address="ed1")]))
         s.commit()
 
@@ -1682,6 +1693,8 @@ class WeakIdentityMapTest(_fixtures.FixtureTest):
             },
         )
         self.mapper_registry.map_imperatively(Address, addresses)
+        gc_collect()
+
         s.add(User(name="ed", address=Address(email_address="ed1")))
         s.commit()
 
@@ -1709,6 +1722,7 @@ class WeakIdentityMapTest(_fixtures.FixtureTest):
         users, User = self.tables.users, self.classes.User
 
         self.mapper_registry.map_imperatively(User, users)
+        gc_collect()
 
         sess = Session(testing.db)
 
@@ -1740,6 +1754,7 @@ class WeakIdentityMapTest(_fixtures.FixtureTest):
         users, User = self.tables.users, self.classes.User
 
         self.mapper_registry.map_imperatively(User, users)
+        gc_collect()
 
         sess = fixture_session()