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

Speed up __eq__ #1310

Merged
merged 6 commits into from
Jul 21, 2024
Merged

Speed up __eq__ #1310

merged 6 commits into from
Jul 21, 2024

Conversation

Tinche
Copy link
Member

@Tinche Tinche commented Jul 19, 2024

Create the __eq__ as a chain of ANDed comparisons, instead of a tuple.

On my computer, a class with two attributes now compares almost twice as fast: 141 ns vs 222 ns.

This will introduce a very slight incompatibility which I think is worth it for us (and dataclasses already introduced): attributes that compare equal by identity but not by value (= NaNs) will behave differently.

Copy link

codspeed-hq bot commented Jul 19, 2024

CodSpeed Performance Report

Merging #1310 will improve performances by 63.24%

Comparing tin/faster-eq (828e055) with main (feb7a4d)

Summary

⚡ 2 improvements
✅ 6 untouched benchmarks

Benchmarks breakdown

Benchmark main tin/faster-eq Change
test_eq_equal 1,136.4 µs 873.3 µs +30.12%
test_eq_unequal 1,098.1 µs 672.7 µs +63.24%

@Tinche Tinche requested a review from hynek July 19, 2024 22:17
src/attr/_make.py Outdated Show resolved Hide resolved
@Tinche Tinche requested a review from hynek July 20, 2024 08:47
changelog.d/1310.change.md Outdated Show resolved Hide resolved
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.

cool thanks! 🏎️

@hynek hynek added this pull request to the merge queue Jul 21, 2024
Merged via the queue into main with commit a8808a2 Jul 21, 2024
21 checks passed
@hynek hynek deleted the tin/faster-eq branch July 21, 2024 10:03
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