Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Speedup uses of object.__setattr__ #898

Merged
merged 3 commits into from Jan 3, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/898.change.rst
@@ -0,0 +1 @@
Speedup instantiation of frozen slotted classes.
10 changes: 5 additions & 5 deletions docs/how-does-it-work.rst
Expand Up @@ -87,16 +87,16 @@ This is (still) slower than a plain assignment:
$ pyperf timeit --rigorous \
-s "import attr; C = attr.make_class('C', ['x', 'y', 'z'], slots=True)" \
"C(1, 2, 3)"
........................................
Median +- std dev: 378 ns +- 12 ns
.........................................
Mean +- std dev: 228 ns +- 18 ns

$ pyperf timeit --rigorous \
-s "import attr; C = attr.make_class('C', ['x', 'y', 'z'], slots=True, frozen=True)" \
"C(1, 2, 3)"
........................................
Median +- std dev: 676 ns +- 16 ns
.........................................
Mean +- std dev: 450 ns +- 26 ns

So on a laptop computer the difference is about 300 nanoseconds (1 second is 1,000,000,000 nanoseconds).
So on a laptop computer the difference is about 230 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: 7 additions & 17 deletions src/attr/_make.py
Expand Up @@ -807,7 +807,7 @@ def _patch_original_class(self):
cls.__attrs_own_setattr__ = False

if not self._has_custom_setattr:
cls.__setattr__ = object.__setattr__
cls.__setattr__ = _obj_setattr

return cls

Expand Down Expand Up @@ -835,7 +835,7 @@ def _create_slots_class(self):
if not self._has_custom_setattr:
for base_cls in self._cls.__bases__:
if base_cls.__dict__.get("__attrs_own_setattr__", False):
cd["__setattr__"] = object.__setattr__
cd["__setattr__"] = _obj_setattr
break

# Traverse the MRO to collect existing slots
Expand Down Expand Up @@ -2159,7 +2159,6 @@ def _make_init(
cache_hash,
base_attr_map,
is_exc,
needs_cached_setattr,
has_cls_on_setattr,
attrs_init,
)
Expand All @@ -2172,7 +2171,7 @@ def _make_init(
if needs_cached_setattr:
# Save the lookup overhead in __init__ if we need to circumvent
# setattr hooks.
globs["_cached_setattr"] = _obj_setattr
globs["_setattr"] = _obj_setattr

init = _make_method(
"__attrs_init__" if attrs_init else "__init__",
Expand All @@ -2189,15 +2188,15 @@ def _setattr(attr_name, value_var, has_on_setattr):
"""
Use the cached object.setattr to set *attr_name* to *value_var*.
"""
return "_setattr('%s', %s)" % (attr_name, value_var)
return "_setattr(self, '%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('%s', %s(%s))" % (
return "_setattr(self, '%s', %s(%s))" % (
attr_name,
_init_converter_pat % (attr_name,),
value_var,
Expand Down Expand Up @@ -2296,7 +2295,6 @@ def _attrs_to_init_script(
cache_hash,
base_attr_map,
is_exc,
needs_cached_setattr,
has_cls_on_setattr,
attrs_init,
):
Expand All @@ -2312,14 +2310,6 @@ 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, self.__class__)"
)

if frozen is True:
if slots is True:
fmt_setter = _setattr
Expand Down Expand Up @@ -2535,7 +2525,7 @@ def fmt_setter_with_converter(
if post_init:
lines.append("self.__attrs_post_init__()")

# because this is set only after __attrs_post_init is called, a crash
# because this is set only after __attrs_post_init__ is called, a crash
# will result if post-init tries to access the hash code. This seemed
# preferable to setting this beforehand, in which case alteration to
# field values during post-init combined with post-init accessing the
Expand All @@ -2544,7 +2534,7 @@ def fmt_setter_with_converter(
if frozen:
if slots:
# if frozen and slots, then _setattr defined above
init_hash_cache = "_setattr('%s', %s)"
init_hash_cache = "_setattr(self, '%s', %s)"
else:
# if frozen and not slots, then _inst_dict defined above
init_hash_cache = "_inst_dict['%s'] = %s"
Expand Down
5 changes: 2 additions & 3 deletions tests/test_functional.py
Expand Up @@ -784,7 +784,6 @@ class D(C):

src = inspect.getsource(D.__init__)

assert "_setattr = _cached_setattr" in src
assert "_setattr('x', x)" in src
assert "_setattr('y', y)" in src
assert "_setattr(self, 'x', x)" in src
assert "_setattr(self, 'y', y)" in src
assert object.__setattr__ != D.__setattr__