Small callcount reductions and refinement for cached queries

zzzeek · zzzeek · commit 903b18828461 · 2020-05-25T22:36:44.000-04:00
baked wasn't using the new one()/first()/one_or_none() methods,
fixed that.

loading._instance_processor() can skip setting up the
quick populators every time because it can cache the getters.

Callcounts have gone below what 1.3 does for the
test_baked_query performance suite, however runtime for
continued inexplicable reasons has not :(.   still suspecting
the result tuples but this seems so hard to believe.

Change-Id: Ifbca04834d27350e0fa82cb8512e66112abc8729
diff --git a/lib/sqlalchemy/ext/baked.py b/lib/sqlalchemy/ext/baked.py
@@ -383,7 +383,7 @@ def __str__(self):
         return str(self._as_query())
 
     def __iter__(self):
-        return iter(self._iter())
+        return self._iter().__iter__()
 
     def _iter(self):
         bq = self.bq
@@ -463,36 +463,23 @@ def first(self):
         Equivalent to :meth:`_query.Query.first`.
 
         """
+
         bq = self.bq.with_criteria(lambda q: q.slice(0, 1))
-        ret = list(
+        return (
             bq.for_session(self.session)
             .params(self._params)
             ._using_post_criteria(self._post_criteria)
+            ._iter()
+            .first()
         )
-        if len(ret) > 0:
-            return ret[0]
-        else:
-            return None
 
     def one(self):
         """Return exactly one result or raise an exception.
 
         Equivalent to :meth:`_query.Query.one`.
 
         """
-        try:
-            ret = self.one_or_none()
-        except orm_exc.MultipleResultsFound as err:
-            util.raise_(
-                orm_exc.MultipleResultsFound(
-                    "Multiple rows were found for one()"
-                ),
-                replace_context=err,
-            )
-        else:
-            if ret is None:
-                raise orm_exc.NoResultFound("No row was found for one()")
-            return ret
+        return self._iter().one()
 
     def one_or_none(self):
         """Return one or zero results, or raise an exception for multiple
@@ -503,17 +490,7 @@ def one_or_none(self):
         .. versionadded:: 1.0.9
 
         """
-        ret = list(self)
-
-        l = len(ret)
-        if l == 1:
-            return ret[0]
-        elif l == 0:
-            return None
-        else:
-            raise orm_exc.MultipleResultsFound(
-                "Multiple rows were found for one_or_none()"
-            )
+        return self._iter().one_or_none()
 
     def all(self):
         """Return all rows.
diff --git a/lib/sqlalchemy/orm/loading.py b/lib/sqlalchemy/orm/loading.py
@@ -14,8 +14,6 @@
 """
 from __future__ import absolute_import
 
-import collections
-
 from . import attributes
 from . import exc as orm_exc
 from . import path_registry
@@ -588,48 +586,86 @@ def _instance_processor(
 
     identity_class = mapper._identity_class
 
-    populators = collections.defaultdict(list)
+    compile_state = context.compile_state
+
+    populators = {}
 
     props = mapper._prop_set
     if only_load_props is not None:
         props = props.intersection(mapper._props[k] for k in only_load_props)
 
-    quick_populators = path.get(
-        context.attributes, "memoized_setups", _none_set
-    )
+    getters = path.get(compile_state.attributes, "getters", None)
+    if getters is None:
+        # directives given to us by the ColumnLoader.setup_query()
+        # methods.   Turn these directives into getters against the
+        # actual result set.
+        quick_populators = path.get(
+            context.attributes, "memoized_setups", _none_set
+        )
+        cached_populators = {
+            "new": [],
+            "expire": [],
+            "quick": [],
+            "todo": [],
+            "delayed": [],
+            "existing": [],
+            "eager": [],
+        }
 
-    for prop in props:
-        if prop in quick_populators:
-            # this is an inlined path just for column-based attributes.
-            col = quick_populators[prop]
-            if col is _DEFER_FOR_STATE:
-                populators["new"].append(
-                    (prop.key, prop._deferred_column_loader)
-                )
-            elif col is _SET_DEFERRED_EXPIRED:
-                # note that in this path, we are no longer
-                # searching in the result to see if the column might
-                # be present in some unexpected way.
-                populators["expire"].append((prop.key, False))
-            elif col is _RAISE_FOR_STATE:
-                populators["new"].append((prop.key, prop._raise_column_loader))
-            else:
-                getter = None
-                if not getter:
-                    getter = result._getter(col, False)
-                if getter:
-                    populators["quick"].append((prop.key, getter))
-                else:
-                    # fall back to the ColumnProperty itself, which
-                    # will iterate through all of its columns
-                    # to see if one fits
-                    prop.create_row_processor(
-                        context, path, mapper, result, adapter, populators
+        pk_cols = mapper.primary_key
+
+        if adapter:
+            pk_cols = [adapter.columns[c] for c in pk_cols]
+        getters = {
+            "populators": cached_populators,
+            "primary_key_getter": result._tuple_getter(pk_cols),
+        }
+
+        for prop in props:
+            if prop in quick_populators:
+                # this is an inlined path just for column-based attributes.
+                col = quick_populators[prop]
+                if col is _DEFER_FOR_STATE:
+                    cached_populators["new"].append(
+                        (prop.key, prop._deferred_column_loader)
                     )
-        else:
-            prop.create_row_processor(
-                context, path, mapper, result, adapter, populators
-            )
+                elif col is _SET_DEFERRED_EXPIRED:
+                    # note that in this path, we are no longer
+                    # searching in the result to see if the column might
+                    # be present in some unexpected way.
+                    cached_populators["expire"].append((prop.key, False))
+                elif col is _RAISE_FOR_STATE:
+                    cached_populators["new"].append(
+                        (prop.key, prop._raise_column_loader)
+                    )
+                else:
+                    getter = None
+                    if not getter:
+                        getter = result._getter(col, False)
+                    if getter:
+                        cached_populators["quick"].append((prop.key, getter))
+                    else:
+                        # fall back to the ColumnProperty itself, which
+                        # will iterate through all of its columns
+                        # to see if one fits
+                        prop.create_row_processor(
+                            context,
+                            path,
+                            mapper,
+                            result,
+                            adapter,
+                            cached_populators,
+                        )
+            else:
+                cached_populators["todo"].append(prop)
+        path.set(compile_state.attributes, "getters", getters)
+
+    cached_populators = getters["populators"]
+    populators = {k: list(v) for k, v in cached_populators.items()}
+    for prop in cached_populators["todo"]:
+        prop.create_row_processor(
+            context, path, mapper, result, adapter, populators
+        )
 
     propagated_loader_options = context.propagated_loader_options
     load_path = (
@@ -707,11 +743,7 @@ def _instance_processor(
     else:
         refresh_identity_key = None
 
-        pk_cols = mapper.primary_key
-
-        if adapter:
-            pk_cols = [adapter.columns[c] for c in pk_cols]
-        tuple_getter = result._tuple_getter(pk_cols)
+        primary_key_getter = getters["primary_key_getter"]
 
     if mapper.allow_partial_pks:
         is_not_primary_key = _none_set.issuperset
@@ -732,7 +764,11 @@ def _instance(row):
         else:
             # look at the row, see if that identity is in the
             # session, or we have to create a new one
-            identitykey = (identity_class, tuple_getter(row), identity_token)
+            identitykey = (
+                identity_class,
+                primary_key_getter(row),
+                identity_token,
+            )
 
             instance = session_identity_map.get(identitykey)
 
@@ -875,7 +911,7 @@ def _instance(row):
         def ensure_no_pk(row):
             identitykey = (
                 identity_class,
-                tuple_getter(row),
+                primary_key_getter(row),
                 identity_token,
             )
             if not is_not_primary_key(identitykey[1]):
diff --git a/test/ext/test_baked.py b/test/ext/test_baked.py
@@ -180,7 +180,7 @@ def test_one_or_none_multiple_result(self):
 
         assert_raises_message(
             orm_exc.MultipleResultsFound,
-            "Multiple rows were found for one_or_none()",
+            "Multiple rows were found when one or none was required",
             bq(Session()).one_or_none,
         )
 
@@ -192,7 +192,7 @@ def test_one_no_result(self):
 
         assert_raises_message(
             orm_exc.NoResultFound,
-            "No row was found for one()",
+            "No row was found when one was required",
             bq(Session()).one,
         )
 
@@ -213,7 +213,7 @@ def test_one_multiple_result(self):
 
         assert_raises_message(
             orm_exc.MultipleResultsFound,
-            "Multiple rows were found for one()",
+            "Multiple rows were found when exactly one was required",
             bq(Session()).one,
         )
 

Original file line number	Diff line number	Diff line change
`@@ -180,7 +180,7 @@ def test_one_or_none_multiple_result(self):`
`180`	`180`
`181`	`181`	`assert_raises_message(`
`182`	`182`	`orm_exc.MultipleResultsFound,`
`183`		`- "Multiple rows were found for one_or_none()",`
	`183`	`+ "Multiple rows were found when one or none was required",`
`184`	`184`	`bq(Session()).one_or_none,`
`185`	`185`	`)`
`186`	`186`
`@@ -192,7 +192,7 @@ def test_one_no_result(self):`
`192`	`192`
`193`	`193`	`assert_raises_message(`
`194`	`194`	`orm_exc.NoResultFound,`
`195`		`- "No row was found for one()",`
	`195`	`+ "No row was found when one was required",`
`196`	`196`	`bq(Session()).one,`
`197`	`197`	`)`
`198`	`198`
`@@ -213,7 +213,7 @@ def test_one_multiple_result(self):`
`213`	`213`
`214`	`214`	`assert_raises_message(`
`215`	`215`	`orm_exc.MultipleResultsFound,`
`216`		`- "Multiple rows were found for one()",`
	`216`	`+ "Multiple rows were found when exactly one was required",`
`217`	`217`	`bq(Session()).one,`
`218`	`218`	`)`
`219`	`219`