From c8e0d64c894ec9106518f963c39c1a26a66e0038 Mon Sep 17 00:00:00 2001 From: davfsa Date: Tue, 2 Aug 2022 14:51:19 +0200 Subject: [PATCH 1/2] Speedup `_setattr` usage and fix performance regressions --- docs/how-does-it-work.rst | 4 ++-- src/attr/_make.py | 24 +++++++++++++++++------- tests/test_functional.py | 5 +++-- 3 files changed, 22 insertions(+), 11 deletions(-) diff --git a/docs/how-does-it-work.rst b/docs/how-does-it-work.rst index c7b408341..12528ab17 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 977d93d39..8e2a45957 100644 --- a/src/attr/_make.py +++ b/src/attr/_make.py @@ -932,7 +932,7 @@ def slots_setstate(self, state): """ 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) @@ -2099,6 +2099,7 @@ def _make_init( cache_hash, base_attr_map, is_exc, + needs_cached_setattr, has_cls_on_setattr, attrs_init, ) @@ -2111,7 +2112,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__", @@ -2128,7 +2129,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): @@ -2136,7 +2137,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, @@ -2178,6 +2179,7 @@ def _attrs_to_init_script( cache_hash, base_attr_map, is_exc, + needs_cached_setattr, has_cls_on_setattr, attrs_init, ): @@ -2193,6 +2195,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 @@ -2407,7 +2417,7 @@ def fmt_setter_with_converter( 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" @@ -2520,7 +2530,7 @@ def __init__( ) # 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. @@ -2617,7 +2627,7 @@ def __setstate__(self, state): 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 09f504802..d73c4076d 100644 --- a/tests/test_functional.py +++ b/tests/test_functional.py @@ -747,6 +747,7 @@ class D(C): 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__ From f26f939d31471a2235edf62530f70b1876fea80f Mon Sep 17 00:00:00 2001 From: davfsa Date: Tue, 2 Aug 2022 16:36:07 +0200 Subject: [PATCH 2/2] Add changelog file --- changelog.d/991.change.rst | 1 + 1 file changed, 1 insertion(+) 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 000000000..bc9487d1d --- /dev/null +++ b/changelog.d/991.change.rst @@ -0,0 +1 @@ +Fix slight performance regression in classes with custom ``__setattr__`` and speedup even more.