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 _setattr usage and fix slight performance regressions #991

Merged

Conversation

davfsa
Copy link
Contributor

@davfsa davfsa commented Aug 2, 2022

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.

3 arguments performance

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
10 arguments performace

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

Pull Request Check List

  • Added tests for changed code.
    Our CI fails if coverage is not 100%.
  • New features have been added to our Hypothesis testing strategy.
  • Changes or additions to public APIs are reflected in our type stubs (files ending in .pyi).
    • ...and used in the stub test file tests/typing_example.py.
    • If they've been added to attr/__init__.pyi, they've also been re-imported in attrs/__init__.pyi.
  • Updated documentation for changed code.
    • New functions/classes have to be added to docs/api.rst by hand.
    • Changes to the signature of @attr.s() have to be added by hand too.
    • Changed/added classes/methods/functions have appropriate versionadded, versionchanged, or deprecated directives.
      Find the appropriate next version in our __init__.py file.
  • Documentation in .rst files is written using semantic newlines.
  • Changes (and possible deprecations) have news fragments in changelog.d.
  • Consider granting push permissions to the PR branch, so maintainers can fix minor issues themselves without pestering you.

@davfsa davfsa force-pushed the task/fix-performace-regression-frozen-classes branch from 219948a to c8e0d64 Compare August 2, 2022 14:24
@davfsa davfsa changed the title Speedup _setattr usage and fix performance regressions Speedup _setattr usage and fix slight performance regressions Aug 2, 2022
@davfsa
Copy link
Contributor Author

davfsa commented Aug 2, 2022

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 cost 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.

@hynek hynek enabled auto-merge (squash) August 7, 2022 07:50
Copy link
Member

@hynek hynek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I’m gonna have to take your word for it here. 😅

@hynek hynek merged commit a819155 into python-attrs:main Aug 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants