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
(cherry picked from commit b8db80e9ea917e4770c78feff092044d386985c6)

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 0aab04139f1a5ec31d2ca0f15b0cbbc3b106eb47..fbb02d9258c8aef7d6d7a0f9de91d921933cecd8 100644 (file)
--- a/lib/sqlalchemy/sql/elements.py
+++ b/lib/sqlalchemy/sql/elements.py
@@ -248,7 +248,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 51e0ae1578e60d7d4576bd71ee9427105722d06b..5516898a83ffb2d6b0c40a4f5195144e81598cb9 100644 (file)
--- a/lib/sqlalchemy/sql/selectable.py
+++ b/lib/sqlalchemy/sql/selectable.py
@@ -4773,7 +4773,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 624c12ea225c4edc2470ddbc30aaaf508321eb44..c842b593016ece630e75b153ac9a6a0038d885b2 100644 (file)
--- a/test/aaa_profiling/test_memusage.py
+++ b/test/aaa_profiling/test_memusage.py
@@ -355,6 +355,17 @@ class MemUsageTest(EnsureZeroed):
 
         go()
 
+    def test_clone_expression(self):
+
+        root_expr = column("x", Integer) == 12
+        expr = [root_expr]
+
+        @profile_memory()
+        def go():
+            expr[0] = cloned_traverse(expr[0], {}, {})
+
+        go()
+
 
 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 607c0a9edcd4e999f17e418ef4eafa412f189abf..295fd8205f3a9e3cc00f194b6449c626b5004229 100644 (file)
--- a/test/orm/test_session.py
+++ b/test/orm/test_session.py
@@ -1534,14 +1534,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()
@@ -1566,6 +1574,7 @@ class WeakIdentityMapTest(_fixtures.FixtureTest):
 
         s = fixture_session()
         self.mapper_registry.map_imperatively(User, users)
+        gc_collect()
 
         s.add(User(name="ed"))
         s.flush()
@@ -1608,6 +1617,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()
 
@@ -1648,6 +1659,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()
 
@@ -1675,6 +1688,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)
 
@@ -1706,6 +1720,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()