diff options
| -rw-r--r-- | doc/build/changelog/unreleased_13/rel_join.rst | 10 | ||||
| -rw-r--r-- | lib/sqlalchemy/orm/relationships.py | 20 | ||||
| -rw-r--r-- | lib/sqlalchemy/orm/util.py | 1 | ||||
| -rw-r--r-- | lib/sqlalchemy/testing/profiling.py | 7 | ||||
| -rw-r--r-- | test/aaa_profiling/test_orm.py | 91 | ||||
| -rw-r--r-- | test/ext/test_serializer.py | 1 | ||||
| -rw-r--r-- | test/profiles.txt | 16 |
7 files changed, 140 insertions, 6 deletions
diff --git a/doc/build/changelog/unreleased_13/rel_join.rst b/doc/build/changelog/unreleased_13/rel_join.rst new file mode 100644 index 000000000..1f0e29ef1 --- /dev/null +++ b/doc/build/changelog/unreleased_13/rel_join.rst @@ -0,0 +1,10 @@ +.. change:: + :tags: performance, orm + + Identified a performance issue in the system by which a join is constructed + based on a mapped relationship. The clause adaption system would be used + for the majority of join expressions including in the common case where no + adaptation is needed. The conditions under which this adaptation occur + have been refined so that average non-aliased joins along a simple + relationship without a "secondary" table use about 70% less function calls. + diff --git a/lib/sqlalchemy/orm/relationships.py b/lib/sqlalchemy/orm/relationships.py index fc3e78ea1..7ee12ca6c 100644 --- a/lib/sqlalchemy/orm/relationships.py +++ b/lib/sqlalchemy/orm/relationships.py @@ -1044,6 +1044,7 @@ class RelationshipProperty(StrategizedProperty): source_selectable=adapt_from, source_polymorphic=True, of_type_mapper=of_type_mapper, + alias_secondary=True, ) if sj is not None: return pj & sj @@ -2215,12 +2216,18 @@ class RelationshipProperty(StrategizedProperty): dest_polymorphic=False, dest_selectable=None, of_type_mapper=None, + alias_secondary=False, ): + + aliased = False + + if alias_secondary and self.secondary is not None: + aliased = True + if source_selectable is None: if source_polymorphic and self.parent.with_polymorphic: source_selectable = self.parent._with_polymorphic_selectable - aliased = False if dest_selectable is None: dest_selectable = self.entity.selectable if dest_polymorphic and self.mapper.with_polymorphic: @@ -2229,13 +2236,20 @@ class RelationshipProperty(StrategizedProperty): if self._is_self_referential and source_selectable is None: dest_selectable = dest_selectable._anonymous_fromclause() aliased = True - else: + elif dest_selectable is not self.mapper._with_polymorphic_selectable: aliased = True dest_mapper = of_type_mapper or self.mapper single_crit = dest_mapper._single_table_criterion - aliased = aliased or (source_selectable is not None) + aliased = aliased or ( + source_selectable is not None + and ( + source_selectable + is not self.parent._with_polymorphic_selectable + or source_selectable._is_subquery + ) + ) ( primaryjoin, diff --git a/lib/sqlalchemy/orm/util.py b/lib/sqlalchemy/orm/util.py index b96387c08..2f78fa535 100644 --- a/lib/sqlalchemy/orm/util.py +++ b/lib/sqlalchemy/orm/util.py @@ -1035,6 +1035,7 @@ class _ORMJoin(expression.Join): source_polymorphic=True, dest_polymorphic=True, of_type_mapper=right_info.mapper, + alias_secondary=True, ) if sj is not None: diff --git a/lib/sqlalchemy/testing/profiling.py b/lib/sqlalchemy/testing/profiling.py index bfc4997e1..b837d9b5b 100644 --- a/lib/sqlalchemy/testing/profiling.py +++ b/lib/sqlalchemy/testing/profiling.py @@ -221,7 +221,7 @@ class ProfileStatsFile(object): profile_f.close() -def function_call_count(variance=0.05): +def function_call_count(variance=0.05, times=1): """Assert a target for a test case's function call count. The main purpose of this assertion is to detect changes in @@ -234,8 +234,11 @@ def function_call_count(variance=0.05): def decorate(fn): def wrap(*args, **kw): + timerange = range(times) with count_functions(variance=variance): - return fn(*args, **kw) + for time in timerange: + rv = fn(*args, **kw) + return rv return update_wrapper(wrap, fn) diff --git a/test/aaa_profiling/test_orm.py b/test/aaa_profiling/test_orm.py index 209bc02e3..e2df85a33 100644 --- a/test/aaa_profiling/test_orm.py +++ b/test/aaa_profiling/test_orm.py @@ -1,12 +1,16 @@ +from sqlalchemy import and_ from sqlalchemy import ForeignKey from sqlalchemy import inspect from sqlalchemy import Integer +from sqlalchemy import join from sqlalchemy import String from sqlalchemy import testing +from sqlalchemy.orm import aliased from sqlalchemy.orm import Bundle from sqlalchemy.orm import configure_mappers from sqlalchemy.orm import defaultload from sqlalchemy.orm import defer +from sqlalchemy.orm import join as orm_join from sqlalchemy.orm import joinedload from sqlalchemy.orm import Load from sqlalchemy.orm import mapper @@ -818,6 +822,93 @@ class JoinedEagerLoadTest(fixtures.MappedTest): go() +class JoinConditionTest(fixtures.DeclarativeMappedTest): + @classmethod + def setup_classes(cls): + class A(cls.DeclarativeBasic): + __tablename__ = "a" + + id = Column(Integer, primary_key=True) + b_id = Column(ForeignKey("b.id")) + b = relationship("B") + + class B(cls.DeclarativeBasic): + __tablename__ = "b" + + id = Column(Integer, primary_key=True) + d_id = Column(ForeignKey("d.id")) + + class C(cls.DeclarativeBasic): + __tablename__ = "c" + + id = Column(Integer, primary_key=True) + a_id = Column(ForeignKey("a.id")) + d_id = Column(ForeignKey("d.id")) + + class D(cls.DeclarativeBasic): + __tablename__ = "d" + + id = Column(Integer, primary_key=True) + + j = join(B, D, B.d_id == D.id).join(C, C.d_id == D.id) + + A.d = relationship( + "D", + secondary=j, + primaryjoin=and_(A.b_id == B.id, A.id == C.a_id), + secondaryjoin=D.id == B.d_id, + uselist=False, + viewonly=True, + ) + + def test_a_to_b_plain(self): + A, B = self.classes("A", "B") + + # should not use aliasing or adaption so should be cheap + @profiling.function_call_count(times=50) + def go(): + orm_join(A, B, A.b) + + go() + + def test_a_to_b_aliased(self): + A, B = self.classes("A", "B") + + a1 = aliased(A) + + # uses aliasing, therefore adaption which is expensive + @profiling.function_call_count(times=50) + def go(): + orm_join(a1, B, a1.b) + + go() + + def test_a_to_d(self): + A, D = self.classes("A", "D") + + # the join condition between A and D uses a secondary selectable with + # overlap so incurs aliasing, which is expensive, there is also a check + # that determines that this overlap exists which is not currently + # cached + @profiling.function_call_count(times=50) + def go(): + orm_join(A, D, A.d) + + go() + + def test_a_to_d_aliased(self): + A, D = self.classes("A", "D") + + a1 = aliased(A) + + # aliased, uses adaption therefore expensive + @profiling.function_call_count(times=50) + def go(): + orm_join(a1, D, a1.d) + + go() + + class BranchedOptionTest(fixtures.MappedTest): @classmethod def define_tables(cls, metadata): diff --git a/test/ext/test_serializer.py b/test/ext/test_serializer.py index e5490146c..e6c7b717f 100644 --- a/test/ext/test_serializer.py +++ b/test/ext/test_serializer.py @@ -243,7 +243,6 @@ class SerializeTest(AssertsCompiledSQL, fixtures.MappedTest): j2 = serializer.loads(serializer.dumps(j, -1), users.metadata) assert j2.left is j.left assert j2.right is j.right - assert j2._target_adapter._next @testing.exclude( "sqlite", "<=", (3, 5, 9), "id comparison failing on the buildbot" diff --git a/test/profiles.txt b/test/profiles.txt index 493f27698..89c01f5d1 100644 --- a/test/profiles.txt +++ b/test/profiles.txt @@ -543,6 +543,22 @@ test.aaa_profiling.test_orm.DeferOptionsTest.test_defer_many_cols 3.7_postgresql test.aaa_profiling.test_orm.DeferOptionsTest.test_defer_many_cols 3.7_sqlite_pysqlite_dbapiunicode_cextensions 23259 test.aaa_profiling.test_orm.DeferOptionsTest.test_defer_many_cols 3.7_sqlite_pysqlite_dbapiunicode_nocextensions 30268 +# TEST: test.aaa_profiling.test_orm.JoinConditionTest.test_a_to_b_aliased + +test.aaa_profiling.test_orm.JoinConditionTest.test_a_to_b_aliased 3.7_sqlite_pysqlite_dbapiunicode_nocextensions 10313 + +# TEST: test.aaa_profiling.test_orm.JoinConditionTest.test_a_to_b_plain + +test.aaa_profiling.test_orm.JoinConditionTest.test_a_to_b_plain 3.7_sqlite_pysqlite_dbapiunicode_nocextensions 3256 + +# TEST: test.aaa_profiling.test_orm.JoinConditionTest.test_a_to_d + +test.aaa_profiling.test_orm.JoinConditionTest.test_a_to_d 3.7_sqlite_pysqlite_dbapiunicode_nocextensions 106798 + +# TEST: test.aaa_profiling.test_orm.JoinConditionTest.test_a_to_d_aliased + +test.aaa_profiling.test_orm.JoinConditionTest.test_a_to_d_aliased 3.7_sqlite_pysqlite_dbapiunicode_nocextensions 104564 + # TEST: test.aaa_profiling.test_orm.JoinedEagerLoadTest.test_build_query test.aaa_profiling.test_orm.JoinedEagerLoadTest.test_build_query 2.7_mssql_pyodbc_dbapiunicode_nocextensions 439163 |