From a8191556c0aad84125d2c3435c5a2352b2f8a9ba Mon Sep 17 00:00:00 2001 From: davfsa Date: Sun, 7 Aug 2022 09:52:28 +0200 Subject: [PATCH] Speedup `_setattr` usage and fix slight performance regressions (#991) * Speedup `_setattr` usage and fix performance regressions * Add changelog file Co-authored-by: Hynek Schlawack --- changelog.d/991.change.rst | 1 + docs/how-does-it-work.rst | 4 ++-- src/attr/_make.py | 24 +++++++++++++++++------- tests/test_functional.py | 5 +++-- 4 files changed, 23 insertions(+), 11 deletions(-) create mode 100644 changelog.d/991.change.rst diff --git a/changelog.d/991.change.rst b/changelog.d/991.change.rst new file mode 100644 index 00000000..bc9487d1 --- /dev/null +++ b/changelog.d/991.change.rst @@ -0,0 +1 @@ +Fix slight performance regression in classes with custom ``__setattr__`` and speedup even more. diff --git a/docs/how-does-it-work.rst b/docs/how-does-it-work.rst index c7b40834..12528ab1 100644 --- a/docs/how-does-it-work.rst +++ b/docs/how-does-it-work.rst @@ -94,9 +94,9 @@ This is (still) slower than a plain assignment: -s "import attr; C = attr.make_class('C', ['x', 'y', 'z'], slots=True, frozen=True)" \ "C(1, 2, 3)" ......................................... - Mean +- std dev: 450 ns +- 26 ns + Mean +- std dev: 425 ns +- 16 ns -So on a laptop computer the difference is about 230 nanoseconds (1 second is 1,000,000,000 nanoseconds). +So on a laptop computer the difference is about 200 nanoseconds (1 second is 1,000,000,000 nanoseconds). It's certainly something you'll feel in a hot loop but shouldn't matter in normal code. Pick what's more important to you. diff --git a/src/attr/_make.py b/src/attr/_make.py index f3925826..0c2da5ed 100644 --- a/src/attr/_make.py +++ b/src/attr/_make.py @@ -915,7 +915,7 @@ class _ClassBuilder: """ Automatically created by attrs. """ - __bound_setattr = _obj_setattr.__get__(self, Attribute) + __bound_setattr = _obj_setattr.__get__(self) for name, value in zip(state_attr_names, state): __bound_setattr(name, value) @@ -2007,6 +2007,7 @@ def _make_init( cache_hash, base_attr_map, is_exc, + needs_cached_setattr, has_cls_on_setattr, attrs_init, ) @@ -2019,7 +2020,7 @@ def _make_init( if needs_cached_setattr: # Save the lookup overhead in __init__ if we need to circumvent # setattr hooks. - globs["_setattr"] = _obj_setattr + globs["_cached_setattr_get"] = _obj_setattr.__get__ init = _make_method( "__attrs_init__" if attrs_init else "__init__", @@ -2036,7 +2037,7 @@ def _setattr(attr_name, value_var, has_on_setattr): """ Use the cached object.setattr to set *attr_name* to *value_var*. """ - return "_setattr(self, '%s', %s)" % (attr_name, value_var) + return "_setattr('%s', %s)" % (attr_name, value_var) def _setattr_with_converter(attr_name, value_var, has_on_setattr): @@ -2044,7 +2045,7 @@ def _setattr_with_converter(attr_name, value_var, has_on_setattr): Use the cached object.setattr to set *attr_name* to *value_var*, but run its converter first. """ - return "_setattr(self, '%s', %s(%s))" % ( + return "_setattr('%s', %s(%s))" % ( attr_name, _init_converter_pat % (attr_name,), value_var, @@ -2086,6 +2087,7 @@ def _attrs_to_init_script( cache_hash, base_attr_map, is_exc, + needs_cached_setattr, has_cls_on_setattr, attrs_init, ): @@ -2101,6 +2103,14 @@ def _attrs_to_init_script( if pre_init: lines.append("self.__attrs_pre_init__()") + if needs_cached_setattr: + lines.append( + # Circumvent the __setattr__ descriptor to save one lookup per + # assignment. + # Note _setattr will be used again below if cache_hash is True + "_setattr = _cached_setattr_get(self)" + ) + if frozen is True: if slots is True: fmt_setter = _setattr @@ -2315,7 +2325,7 @@ def _attrs_to_init_script( if frozen: if slots: # if frozen and slots, then _setattr defined above - init_hash_cache = "_setattr(self, '%s', %s)" + init_hash_cache = "_setattr('%s', %s)" else: # if frozen and not slots, then _inst_dict defined above init_hash_cache = "_inst_dict['%s'] = %s" @@ -2428,7 +2438,7 @@ class Attribute: ) # Cache this descriptor here to speed things up later. - bound_setattr = _obj_setattr.__get__(self, Attribute) + bound_setattr = _obj_setattr.__get__(self) # Despite the big red warning, people *do* instantiate `Attribute` # themselves. @@ -2525,7 +2535,7 @@ class Attribute: self._setattrs(zip(self.__slots__, state)) def _setattrs(self, name_values_pairs): - bound_setattr = _obj_setattr.__get__(self, Attribute) + bound_setattr = _obj_setattr.__get__(self) for name, value in name_values_pairs: if name != "metadata": bound_setattr(name, value) diff --git a/tests/test_functional.py b/tests/test_functional.py index 74106801..cdbbd523 100644 --- a/tests/test_functional.py +++ b/tests/test_functional.py @@ -745,6 +745,7 @@ class TestFunctional: src = inspect.getsource(D.__init__) - assert "_setattr(self, 'x', x)" in src - assert "_setattr(self, 'y', y)" in src + assert "_setattr = _cached_setattr_get(self)" in src + assert "_setattr('x', x)" in src + assert "_setattr('y', y)" in src assert object.__setattr__ != D.__setattr__