#28860 closed Cleanup/optimization (fixed)
Reduce calls to len()
Reported by: | Дилян Палаузов | Owned by: | nobody |
---|---|---|---|
Component: | Core (Other) | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Ready for checkin | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
if len(x)
is the same as if x
except when len()
is supposed to throw exception when x
is not iterable.
len(x)
can return either zero or a positive integer, hence if len(x) < 1
is the same as if len(x) == 0
<=> if not x
.
Attachments (1)
Change History (11)
by , 7 years ago
Attachment: | less-len.patch added |
---|
comment:1 by , 7 years ago
Component: | Uncategorized → Core (Other) |
---|---|
Triage Stage: | Unreviewed → Ready for checkin |
comment:2 by , 7 years ago
Even more places:
diff --git a/django/contrib/admin/filters.py b/django/contrib/admin/filters.py --- a/django/contrib/admin/filters.py +++ b/django/contrib/admin/filters.py @@ -77,7 +77,7 @@ class SimpleListFilter(ListFilter): self.lookup_choices = list(lookup_choices) def has_output(self): - return len(self.lookup_choices) > 0 + return bool(self.lookup_choices) def value(self): """ diff --git a/django/db/backends/oracle/creation.py b/django/db/backends/oracle/creation.py --- a/django/db/backends/oracle/creation.py +++ b/django/db/backends/oracle/creation.py @@ -268,7 +268,7 @@ class DatabaseCreation(BaseDatabaseCreation): """ try: # Statement can fail when acceptable_ora_err is not None - allow_quiet_fail = acceptable_ora_err is not None and len(acceptable_ora_err) > 0 + allow_quiet_fail = bool(acceptable_ora_err) self._execute_statements(cursor, statements, parameters, verbosity, allow_quiet_fail=allow_quiet_fa return True except DatabaseError as err: diff --git a/django/db/migrations/questioner.py b/django/db/migrations/questioner.py --- a/django/db/migrations/questioner.py +++ b/django/db/migrations/questioner.py @@ -85,7 +85,7 @@ class InteractiveMigrationQuestioner(MigrationQuestioner): result = input("%s " % question) if not result and default is not None: return default - while len(result) < 1 or result[0].lower() not in "yn": + while not result or result[0].lower() not in "yn": result = input("Please answer yes or no: ") return result[0].lower() == "y" diff --git a/django/db/models/base.py b/django/db/models/base.py --- a/django/db/models/base.py +++ b/django/db/models/base.py @@ -736,7 +736,7 @@ class Model(metaclass=ModelBase): """ using = using or router.db_for_write(self.__class__, instance=self) assert not (force_insert and (force_update or update_fields)) - assert update_fields is None or len(update_fields) > 0 + assert update_fields is None or update_fields cls = origin = self.__class__ # Skip proxies, but keep the origin as the proxy model. if cls._meta.proxy:
comment:3 by , 7 years ago
Some changes in the patch are indeed improvements, but not all of them.
Regarding the comment above, I find that return len(self.lookup_choices) > 0
is much more explicit than return bool(self.lookup_choices)
as well as much faster:
>>> import timeit >>> timeit.timeit('len([]) > 0') 0.0808453639911022 >>> timeit.timeit('bool([])') 0.1313885069976095
comment:4 by , 7 years ago
The performance hit is specific to extra bool
calls.
if <something>
is faster than if len(<something>)
.
comment:5 by , 7 years ago
I didn't know that len()
is an O(1) operation for many types.
So it looks like for that case, the fastest option would be True if self.lookup_choices else False
.
>>> timeit.timeit('True if [1] else False') 0.06975528400289477 >>> timeit.timeit('bool([1])') 0.18175133999829995 >>> timeit.timeit('len([1]) > 0') 0.12322649999987334
Is that optimizing at the expense of readability?
comment:6 by , 7 years ago
len()
faster than bool()
on lists seems to be deficiency in cpython. I wrote a report https://bugs.python.org/issue32180.
I think for the moment len(self.lookup_choices) > 0
can be left as it is with appropriate comment (e.g. reference to this ticket), so that one does not come to the idea to investigate whether there is place for optimizations on that place.
comment:7 by , 7 years ago
Yes, in my opinion True if ... else False
for performance is the worse option there. I agree with Дилян.
comment:8 by , 7 years ago
Another option would be to change the documentation for the return type of has_output: instead of boolean, it returns a value, that can be converted to boolean equivalent to the original return value. In fact, internally has_output is called only on one place.
In db/backends/oracle/creation.py the cast to allow_quiet_fail = bool (acceptable_ora_err)
is superfluous, as the called _execute_statements(... allow_quiet_fail=allow_quiet_fail)}}} just calls if not allow_quiet_fail...
. Hence calling self._execute_statements(cursor, statements, parameters, verbosity, allow_quiet_fail=acceptable_ora_err)
and deleting the local variable allow_quiet_fail
is reasonable.
In django/db/backends/postgresql/operations.py the code can be shortened to column_name = sequence_info['column'] or 'id'
.
comment:10 by , 7 years ago
My comments for django/db/backends/oracle/creation.py and django/db/backends/postgresql/operations.py weren't tackled.
PR from the patch.