Skip to content

Commit

Permalink
Speedup _setattr usage and fix slight performance regressions (#991)
Browse files Browse the repository at this point in the history
* Speedup `_setattr` usage and fix performance regressions

* Add changelog file

Co-authored-by: Hynek Schlawack <hs@ox.cx>
  • Loading branch information
davfsa and hynek committed Aug 7, 2022
1 parent 983c2c4 commit a819155
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 11 deletions.
1 change: 1 addition & 0 deletions changelog.d/991.change.rst
@@ -0,0 +1 @@
Fix slight performance regression in classes with custom ``__setattr__`` and speedup even more.
4 changes: 2 additions & 2 deletions docs/how-does-it-work.rst
Expand Up @@ -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.

Expand Down
24 changes: 17 additions & 7 deletions src/attr/_make.py
Expand Up @@ -915,7 +915,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)

Expand Down Expand Up @@ -2007,6 +2007,7 @@ def _make_init(
cache_hash,
base_attr_map,
is_exc,
needs_cached_setattr,
has_cls_on_setattr,
attrs_init,
)
Expand All @@ -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__",
Expand All @@ -2036,15 +2037,15 @@ 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):
"""
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,
Expand Down Expand Up @@ -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,
):
Expand All @@ -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
Expand Down Expand Up @@ -2315,7 +2325,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"
Expand Down Expand Up @@ -2428,7 +2438,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.
Expand Down Expand Up @@ -2525,7 +2535,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)
Expand Down
5 changes: 3 additions & 2 deletions tests/test_functional.py
Expand Up @@ -745,6 +745,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__

0 comments on commit a819155

Please sign in to comment.