Repository: python-attrs/attrs
Pull request #898 (2021-12-30 12:59:48)

Speedup uses of `object.__setattr__`
# Summary

I was playing around seeing if I could speedup the creation of frozen slot classes for a project of mine since we instantiate a lot of attr classes. Ideally they would be frozen, but we didnt go with it because of the speed implications it had.

To try and speed it up I tried different things:
- Make use of `objects.__setattr__` during the whole of the init and then place the frozen one at the end
  - The idea behind this is to use the default `__setattr__` during the whole `__init__` function and then setting it to the frozen one after, which I thought would bring speedups as well as improve the UX, since they can could set attributes normally in the init. This unfortunately didnt work nicely with slots and slowed it down even further when attempting a workaround
- Playing around a bit with python internals and trying to figure out the ideal way to implement this
  - This is the change this PR adds does

Overall I wasn't really successful, but I did find this small performance gain of "1.1x", which isnt much, but I think it might be worth having.

Bellow you can see the pypref benchmarks on my laptop (to simulate the ones that were in the documentation)

### Before (attrs == 21.4.0)

```bash
$ pyperf timeit --rigorous -s "import attr; C = attr.make_class('C', ['x', 'y', 'z'], slots=True, frozen=True)" "C(1, 2, 3)"
.........................................
Mean +- std dev: 499 ns +- 26 ns
```

### After
```bash
$ pyperf timeit --rigorous -s "import attr; C = attr.make_class('C', ['x', 'y', 'z'], slots=True, frozen=True)" "C(1, 2, 3)"
.........................................
Mean +- std dev: 450 ns +- 17 ns
```


# Pull Request Check List

<!--
This is just a friendly reminder about the most common mistakes.
Please make sure that you tick all boxes.
But please read our [contribution guide](https://github.com/python-attrs/attrs/blob/main/.github/CONTRIBUTING.md) at least once, it will save you unnecessary review cycles!

If an item doesn't apply to your pull request, **check it anyway** to make it apparent that there's nothing left to do.
If your pull request is a documentation fix or a trivial typo, feel free to delete the whole thing.
-->

- [x] Added **tests** for changed code.
  Our CI fails if coverage is not 100%.
- [x] New features have been added to our [Hypothesis testing strategy](https://github.com/python-attrs/attrs/blob/main/tests/strategies.py).
- [x] Changes or additions to public APIs are reflected in our type stubs (files ending in ``.pyi``).
    - [x] ...and used in the stub test file `tests/typing_example.py`.
    - [x] If they've been added to `attr/__init__.pyi`, they've *also* been re-imported in `attrs/__init__.pyi`.
- [x] Updated **documentation** for changed code.
    - [x] New functions/classes have to be added to `docs/api.rst` by hand.
    - [x] Changes to the signature of `@attr.s()` have to be added by hand too.
    - [x] Changed/added classes/methods/functions have appropriate `versionadded`, `versionchanged`, or `deprecated` [directives](http://www.sphinx-doc.org/en/stable/markup/para.html#directive-versionadded).
          Find the appropriate next version in our [``__init__.py``](https://github.com/python-attrs/attrs/blob/main/src/attr/__init__.py) file.
- [x] Documentation in `.rst` files is written using [semantic newlines](https://rhodesmill.org/brandon/2012/one-sentence-per-line/).
- [x] Changes (and possible deprecations) have news fragments in [`changelog.d`](https://github.com/python-attrs/attrs/blob/main/changelog.d).



hynek: Hmmm 10% more performance is nice and he solution is simpler too, which is nice!

Calling @Tinche who seems to be the daddy of the old code – any edge cases we're missing here?
Tinche: This essentially saves us an attribute lookup at the cost of an additional argument in a couple function calls?

If the tests pass, LGTM.
hynek: @davfsa can you rebase please? you didn't give us write permissions to your branch
davfsa: Rebased and turned on edits in case anything else is needed :)

Patch:
diff --git a/changelog.d/898.change.rst b/changelog.d/898.change.rst
new file mode 100644
index 0000000..bbd2d8b
--- /dev/null
+++ b/changelog.d/898.change.rst
@@ -0,0 +1,1 @@
+Speedup instantiation of frozen slotted classes.
diff --git a/docs/how-does-it-work.rst b/docs/how-does-it-work.rst
index f899740..c7b4083 100644
--- a/docs/how-does-it-work.rst
+++ b/docs/how-does-it-work.rst
@@ -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.
 
diff --git a/src/attr/_make.py b/src/attr/_make.py
index 072d5ff..915c5e6 100644
--- a/src/attr/_make.py
+++ b/src/attr/_make.py
@@ -807,7 +807,7 @@ class _ClassBuilder(object):
             cls.__attrs_own_setattr__ = False
 
             if not self._has_custom_setattr:
-                cls.__setattr__ = object.__setattr__
+                cls.__setattr__ = _obj_setattr
 
         return cls
 
@@ -835,7 +835,7 @@ class _ClassBuilder(object):
             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
@@ -2159,7 +2159,6 @@ def _make_init(
         cache_hash,
         base_attr_map,
         is_exc,
-        needs_cached_setattr,
         has_cls_on_setattr,
         attrs_init,
     )
@@ -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__",
@@ -2189,7 +2188,7 @@ 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):
@@ -2197,7 +2196,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('%s', %s(%s))" % (
+    return "_setattr(self, '%s', %s(%s))" % (
         attr_name,
         _init_converter_pat % (attr_name,),
         value_var,
@@ -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,
 ):
@@ -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
@@ -2535,7 +2525,7 @@ def _attrs_to_init_script(
     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
@@ -2544,7 +2534,7 @@ def _attrs_to_init_script(
         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"


Test patch:
diff --git a/tests/test_functional.py b/tests/test_functional.py
index 9b6a27e..6bb989f 100644
--- a/tests/test_functional.py
+++ b/tests/test_functional.py
@@ -784,7 +784,6 @@ class TestFunctional(object):
 
         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__