Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Fixed #20950 -- Instantiate OrderedDict() only when needed

The use of OrderedDict (even an empty one) was surprisingly slow. By
initializing OrderedDict only when needed it is possible to save
non-trivial amount of computing time (Model.save() is around 30% faster
for example).

This commit targetted sql.Query only, there are likely other places
which could use similar optimizations.
  • Loading branch information...
commit ff723d894d9272ea721d1996432ffc806c2b8180 1 parent 886bb9d
@akaariai akaariai authored akaariai committed
Showing with 47 additions and 22 deletions.
  1. +2 −2 django/db/models/sql/compiler.py
  2. +45 −20 django/db/models/sql/query.py
View
4 django/db/models/sql/compiler.py
@@ -391,7 +391,7 @@ def get_ordering(self):
if not distinct or elt in select_aliases:
result.append('%s %s' % (elt, order))
group_by.append((elt, []))
- elif get_order_dir(field)[0] not in self.query.extra:
+ elif not self.query._extra or get_order_dir(field)[0] not in self.query._extra:
# 'col' is of the form 'field' or 'field1__field2' or
# '-field1__field2__field', etc.
for table, cols, order in self.find_ordering_name(field,
@@ -987,7 +987,7 @@ def pre_sql_setup(self):
# We need to use a sub-select in the where clause to filter on things
# from other tables.
query = self.query.clone(klass=Query)
- query.extra = {}
+ query._extra = {}
query.select = []
query.add_fields([query.get_meta().pk.name])
# Recheck the count - it is possible that fiddling with the select
View
65 django/db/models/sql/query.py
@@ -143,7 +143,10 @@ def __init__(self, model, where=WhereNode):
self.select_related = False
# SQL aggregate-related attributes
- self.aggregates = OrderedDict() # Maps alias -> SQL aggregate function
+ # The _aggregates will be an OrderedDict when used. Due to the cost
+ # of creating OrderedDict this attribute is created lazily (in
+ # self.aggregates property).
+ self._aggregates = None # Maps alias -> SQL aggregate function
self.aggregate_select_mask = None
self._aggregate_select_cache = None
@@ -153,7 +156,9 @@ def __init__(self, model, where=WhereNode):
# These are for extensions. The contents are more or less appended
# verbatim to the appropriate clause.
- self.extra = OrderedDict() # Maps col_alias -> (col_sql, params).
+ # The _extra attribute is an OrderedDict, lazily created similarly to
+ # .aggregates
+ self._extra = None # Maps col_alias -> (col_sql, params).
self.extra_select_mask = None
self._extra_select_cache = None
@@ -165,6 +170,18 @@ def __init__(self, model, where=WhereNode):
# load.
self.deferred_loading = (set(), True)
+ @property
+ def extra(self):
+ if self._extra is None:
+ self._extra = OrderedDict()
+ return self._extra
+
+ @property
+ def aggregates(self):
+ if self._aggregates is None:
+ self._aggregates = OrderedDict()
+ return self._aggregates
+
def __str__(self):
"""
Returns the query as a string of SQL with the parameter values
@@ -245,7 +262,7 @@ def clone(self, klass=None, memo=None, **kwargs):
obj.select_for_update_nowait = self.select_for_update_nowait
obj.select_related = self.select_related
obj.related_select_cols = []
- obj.aggregates = self.aggregates.copy()
+ obj._aggregates = self._aggregates.copy() if self._aggregates is not None else None
if self.aggregate_select_mask is None:
obj.aggregate_select_mask = None
else:
@@ -257,7 +274,7 @@ def clone(self, klass=None, memo=None, **kwargs):
# used.
obj._aggregate_select_cache = None
obj.max_depth = self.max_depth
- obj.extra = self.extra.copy()
+ obj._extra = self._extra.copy() if self._extra is not None else None
if self.extra_select_mask is None:
obj.extra_select_mask = None
else:
@@ -344,7 +361,7 @@ def get_aggregation(self, using, force_subq=False):
# and move them to the outer AggregateQuery.
for alias, aggregate in self.aggregate_select.items():
if aggregate.is_summary:
- query.aggregate_select[alias] = aggregate.relabeled_clone(relabels)
+ query.aggregates[alias] = aggregate.relabeled_clone(relabels)
del obj.aggregate_select[alias]
try:
@@ -358,7 +375,7 @@ def get_aggregation(self, using, force_subq=False):
query = self
self.select = []
self.default_cols = False
- self.extra = {}
+ self._extra = {}
self.remove_inherited_models()
query.clear_ordering(True)
@@ -527,7 +544,7 @@ def combine(self, rhs, connector):
# It would be nice to be able to handle this, but the queries don't
# really make sense (or return consistent value sets). Not worth
# the extra complexity when you can write a real query instead.
- if self.extra and rhs.extra:
+ if self._extra and rhs._extra:
raise ValueError("When merging querysets using 'or', you "
"cannot have extra(select=...) on both sides.")
self.extra.update(rhs.extra)
@@ -756,8 +773,9 @@ def relabel_column(col):
self.group_by = [relabel_column(col) for col in self.group_by]
self.select = [SelectInfo(relabel_column(s.col), s.field)
for s in self.select]
- self.aggregates = OrderedDict(
- (key, relabel_column(col)) for key, col in self.aggregates.items())
+ if self._aggregates:
+ self._aggregates = OrderedDict(
+ (key, relabel_column(col)) for key, col in self._aggregates.items())
# 2. Rename the alias in the internal table/alias datastructures.
for ident, aliases in self.join_map.items():
@@ -967,7 +985,7 @@ def add_aggregate(self, aggregate, model, alias, is_summary):
"""
opts = model._meta
field_list = aggregate.lookup.split(LOOKUP_SEP)
- if len(field_list) == 1 and aggregate.lookup in self.aggregates:
+ if len(field_list) == 1 and self._aggregates and aggregate.lookup in self.aggregates:
# Aggregate is over an annotation
field_name = field_list[0]
col = field_name
@@ -1049,7 +1067,7 @@ def solve_lookup_type(self, lookup):
lookup_parts = lookup.split(LOOKUP_SEP)
num_parts = len(lookup_parts)
if (len(lookup_parts) > 1 and lookup_parts[-1] in self.query_terms
- and lookup not in self.aggregates):
+ and (not self._aggregates or lookup not in self._aggregates)):
# Traverse the lookup query to distinguish related fields from
# lookup types.
lookup_model = self.model
@@ -1108,10 +1126,11 @@ def build_filter(self, filter_expr, branch_negated=False, current_negated=False,
value, lookup_type = self.prepare_lookup_value(value, lookup_type, can_reuse)
clause = self.where_class()
- for alias, aggregate in self.aggregates.items():
- if alias in (parts[0], LOOKUP_SEP.join(parts)):
- clause.add((aggregate, lookup_type, value), AND)
- return clause
+ if self._aggregates:
+ for alias, aggregate in self.aggregates.items():
+ if alias in (parts[0], LOOKUP_SEP.join(parts)):
+ clause.add((aggregate, lookup_type, value), AND)
+ return clause
opts = self.get_meta()
alias = self.get_initial_alias()
@@ -1170,6 +1189,8 @@ def need_having(self, obj):
Returns whether or not all elements of this q_object need to be put
together in the HAVING clause.
"""
+ if not self._aggregates:
+ return False
if not isinstance(obj, Node):
return (refs_aggregate(obj[0].split(LOOKUP_SEP), self.aggregates)
or (hasattr(obj[1], 'contains_aggregate')
@@ -1632,7 +1653,7 @@ def add_count_column(self):
# Set only aggregate to be the count column.
# Clear out the select cache to reflect the new unmasked aggregates.
- self.aggregates = {None: count}
+ self._aggregates = {None: count}
self.set_aggregate_mask(None)
self.group_by = None
@@ -1781,7 +1802,8 @@ def set_extra_mask(self, names):
self.extra_select_mask = set(names)
self._extra_select_cache = None
- def _aggregate_select(self):
+ @property
+ def aggregate_select(self):
"""The OrderedDict of aggregate columns that are not masked, and should
be used in the SELECT clause.
@@ -1789,6 +1811,8 @@ def _aggregate_select(self):
"""
if self._aggregate_select_cache is not None:
return self._aggregate_select_cache
+ elif not self._aggregates:
+ return {}
elif self.aggregate_select_mask is not None:
self._aggregate_select_cache = OrderedDict(
(k, v) for k, v in self.aggregates.items()
@@ -1797,11 +1821,13 @@ def _aggregate_select(self):
return self._aggregate_select_cache
else:
return self.aggregates
- aggregate_select = property(_aggregate_select)
- def _extra_select(self):
+ @property
+ def extra_select(self):
if self._extra_select_cache is not None:
return self._extra_select_cache
+ if not self._extra:
+ return {}
elif self.extra_select_mask is not None:
self._extra_select_cache = OrderedDict(
(k, v) for k, v in self.extra.items()
@@ -1810,7 +1836,6 @@ def _extra_select(self):
return self._extra_select_cache
else:
return self.extra
- extra_select = property(_extra_select)
def trim_start(self, names_with_path):
"""
Please sign in to comment.
Something went wrong with that request. Please try again.