Performance: leverage dict comprehension in PEP-0274

PEP-0274 introduced dict comprehensions to replace dict constructor
with a sequence of length-2 sequences, these are benefits copied
from [1]:
  The dictionary constructor approach has two distinct disadvantages
  from the proposed syntax though.  First, it isn't as legible as a
  dict comprehension.  Second, it forces the programmer to create an
  in-core list object first, which could be expensive.
Magnum does not support python 2.6, we can leverage this.
There is deep dive about PEP-0274[2] and basic tests about
performance[3].
Note: This commit doesn't handle dict constructor with kwagrs.
This commit also adds a hacking rule.

[1]http://legacy.python.org/dev/peps/pep-0274/
[2]http://doughellmann.com/2012/11/12/the-performance-impact-of-using
-dict-instead-of-in-cpython-2-7-2.html
[3]http://paste.openstack.org/show/480757/

Change-Id: I61992fa428d6760449afe3754b02506336e8b421
changes/50/252950/3
houming-wang 7 years ago
parent e7b27cd47e
commit 98875345c2
  1. 2
      HACKING.rst
  2. 8
      magnum/api/controllers/base.py
  3. 2
      magnum/cmd/template_manage.py
  4. 10
      magnum/hacking/checks.py
  5. 6
      magnum/objects/base.py
  6. 2
      magnum/tests/unit/api/utils.py
  7. 28
      magnum/tests/unit/test_hacking.py

@ -22,3 +22,5 @@ Magnum Specific Commandments
assertIsInstance(A, B).
- [M334] Change assertTrue/False(A in/not in B, message) to the more specific
assertIn/NotIn(A, B, message)
- [M336] Must use a dict comprehension instead of a dict constructor
with a sequence of key-value pairs.

@ -31,10 +31,10 @@ class APIBase(wtypes.Base):
def as_dict(self):
"""Render this object as a dict of its fields."""
return dict((k, getattr(self, k))
for k in self.fields
if hasattr(self, k) and
getattr(self, k) != wsme.Unset)
return {k: getattr(self, k)
for k in self.fields
if hasattr(self, k) and
getattr(self, k) != wsme.Unset}
def unset_fields_except(self, except_list=None):
"""Unset fields so they don't appear in the message body.

@ -39,7 +39,7 @@ def print_rows(rows):
fields.append('path')
field_labels.append('Template Path')
formatters = dict((key, operator.itemgetter(key)) for key in fields)
formatters = {key: operator.itemgetter(key) for key in fields}
cliutils.print_list(rows, fields,
formatters=formatters,

@ -50,6 +50,7 @@ asse_equal_with_is_not_none_re = re.compile(
assert_true_isinstance_re = re.compile(
r"(.)*assertTrue\(isinstance\((\w|\.|\'|\"|\[|\])+, "
"(\w|\.|\'|\"|\[|\])+\)\)")
dict_constructor_with_list_copy_re = re.compile(r".*\bdict\((\[)?(\(|\[)")
def check_policy_enforce_decorator(logical_line,
@ -136,6 +137,14 @@ def use_timeutils_utcnow(logical_line, filename):
yield (pos, msg % f)
def dict_constructor_with_list_copy(logical_line):
msg = ("M336: Must use a dict comprehension instead of a dict constructor"
" with a sequence of key-value pairs."
)
if dict_constructor_with_list_copy_re.match(logical_line):
yield (0, msg)
def factory(register):
register(check_policy_enforce_decorator)
register(no_mutable_default_args)
@ -145,3 +154,4 @@ def factory(register):
register(assert_true_isinstance)
register(assert_equal_in)
register(use_timeutils_utcnow)
register(dict_constructor_with_list_copy)

@ -43,9 +43,9 @@ class MagnumObject(ovoo_base.VersionedObject):
OBJ_PROJECT_NAMESPACE = 'magnum'
def as_dict(self):
return dict((k, getattr(self, k))
for k in self.fields
if hasattr(self, k))
return {k: getattr(self, k)
for k in self.fields
if hasattr(self, k)}
class MagnumObjectDictCompat(ovoo_base.VersionedObjectDictCompat):

@ -28,7 +28,7 @@ from magnum.tests.unit.db import utils
def remove_internal(values, internal):
# NOTE(yuriyz): internal attributes should not be posted, except uuid
int_attr = [attr.lstrip('/') for attr in internal if attr != '/uuid']
return dict([(k, v) for (k, v) in values.items() if k not in int_attr])
return {k: v for (k, v) in values.items() if k not in int_attr}
def baymodel_post_data(**kw):

@ -204,3 +204,31 @@ class HackingTestCase(base.TestCase):
code = "aaa"
self._assert_has_no_errors(code, check, filename)
def test_dict_constructor_with_list_copy(self):
self.assertEqual(1, len(list(checks.dict_constructor_with_list_copy(
" dict([(i, connect_info[i])"))))
self.assertEqual(1, len(list(checks.dict_constructor_with_list_copy(
" attrs = dict([(k, _from_json(v))"))))
self.assertEqual(1, len(list(checks.dict_constructor_with_list_copy(
" type_names = dict((value, key) for key, value in"))))
self.assertEqual(1, len(list(checks.dict_constructor_with_list_copy(
" dict((value, key) for key, value in"))))
self.assertEqual(1, len(list(checks.dict_constructor_with_list_copy(
"foo(param=dict((k, v) for k, v in bar.items()))"))))
self.assertEqual(1, len(list(checks.dict_constructor_with_list_copy(
" dict([[i,i] for i in range(3)])"))))
self.assertEqual(1, len(list(checks.dict_constructor_with_list_copy(
" dd = dict([i,i] for i in range(3))"))))
self.assertEqual(0, len(list(checks.dict_constructor_with_list_copy(
" create_kwargs = dict(snapshot=snapshot,"))))
self.assertEqual(0, len(list(checks.dict_constructor_with_list_copy(
" self._render_dict(xml, data_el, data.__dict__)"))))

Loading…
Cancel
Save