Repository: python-attrs/attrs
Pull request #991 (2022-08-02 14:23:48)

Speedup `_setattr` usage and fix slight performance regressions
# Summary

Fix slight performance regression of #898.

I only noticed this after upgrading to 22.1.0, but the PR i made a while back to speedup the instantiation of frozen classes did so, but at a cost of not scaling correctly the more attributes that need to be set

Didnt want to just revert the commit, so played around a bit and found some optimizations. which are in this pr. They include (partially) going back to some of the old code, but with some optimizations that makes it both scale better and perform better in simple tests, ending up being a win win. Sorry for the slight performance regression this issue caused. It didnt cross to me it wouldnt scale well.

<details>
  <summary>3 arguments performance</summary>

### `attrs-21.4.0`
```
$ 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
```

### `attrs-22.1.0`
```
$ 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
```

### `this pr`
```
$ pyperf timeit --rigorous -s "import attr; C = attr.make_class('C', ['a', 'b', 'c'], slots=True, frozen=True)" "C(1, 2, 3)"
.........................................
Mean +- std dev: 425 ns +- 16 ns
```
</details>

<details>
  <summary>10 arguments performace</summary>

### `attrs-21.4.0`
```
$ pyperf timeit --rigorous -s "import attr; C = attr.make_class('C', ['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j'], slots=True, frozen=True)" "C(1, 2, 3, 4, 5, 6, 7, 8, 9, 10)"
.........................................
Mean +- std dev: 1.01 us +- 0.04 us
```

### `attrs-22.1.0`
```
$ pyperf timeit --rigorous -s "import attr; C = attr.make_class('C', ['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j'], slots=True, frozen=True)" "C(1, 2, 3, 4, 5, 6, 7, 8, 9, 10)"
.........................................
Mean +- std dev: 1.18 us +- 0.04 us
```

### `this pr`
```
$ pyperf timeit --rigorous -s "import attr; C = attr.make_class('C', ['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j'], slots=True, frozen=True)" "C(1, 2, 3, 4, 5, 6, 7, 8, 9, 10)"
.........................................
Mean +- std dev: 941 ns +- 45 ns
```
</details>

# Pull Request Check List

- [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).
- [x] Consider granting [push permissions to the PR branch](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/allowing-changes-to-a-pull-request-branch-created-from-a-fork), so maintainers can fix minor issues themselves without pestering you.

davfsa: Also worth noting that frozen classes don't scale well when it comes to attribute setting. When comparing setting 3 attributes in frozen and unfrozen slotted classes, you get a difference of about 200 nanoseconds. When you expand the test to 10 attributes, you get a difference of almost 500! (943 vs 445). 

Looks like the performance of making so many calls is not ideal, but I can't think of a better way of doing this without really stretching the limits of python.

Patch:
diff --git a/changelog.d/991.change.rst b/changelog.d/991.change.rst
new file mode 100644
index 0000000..bc9487d
--- /dev/null
+++ b/changelog.d/991.change.rst
@@ -0,0 +1,1 @@
+Fix slight performance regression in classes with custom ``__setattr__`` and speedup even more.
diff --git a/docs/how-does-it-work.rst b/docs/how-does-it-work.rst
index c7b4083..12528ab 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 f392582..0c2da5e 100644
--- a/src/attr/_make.py
+++ b/src/attr/_make.py
@@ -915,7 +915,7 @@ class _ClassBuilder:
             """
             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)
 
@@ -2007,6 +2007,7 @@ def _make_init(
         cache_hash,
         base_attr_map,
         is_exc,
+        needs_cached_setattr,
         has_cls_on_setattr,
         attrs_init,
     )
@@ -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__",
@@ -2036,7 +2037,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):
@@ -2044,7 +2045,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,
@@ -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,
 ):
@@ -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
@@ -2315,7 +2325,7 @@ def _attrs_to_init_script(
         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"
@@ -2428,7 +2438,7 @@ class Attribute:
         )
 
         # 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.
@@ -2525,7 +2535,7 @@ class Attribute:
         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)


Test patch:
diff --git a/tests/test_functional.py b/tests/test_functional.py
index 7410680..cdbbd52 100644
--- a/tests/test_functional.py
+++ b/tests/test_functional.py
@@ -745,6 +745,7 @@ class TestFunctional:
 
         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__