Skip to content

Commit

Permalink
Speedup _setattr usage and fix performance regressions
Browse files Browse the repository at this point in the history
  • Loading branch information
davfsa committed Aug 2, 2022
1 parent 107367d commit c8e0d64
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 11 deletions.
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 @@ -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)

Expand Down Expand Up @@ -2099,6 +2099,7 @@ def _make_init(
cache_hash,
base_attr_map,
is_exc,
needs_cached_setattr,
has_cls_on_setattr,
attrs_init,
)
Expand All @@ -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__",
Expand All @@ -2128,15 +2129,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 @@ -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,
):
Expand All @@ -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
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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)
Expand Down
5 changes: 3 additions & 2 deletions tests/test_functional.py
Expand Up @@ -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__

0 comments on commit c8e0d64

Please sign in to comment.