biqqles / dataclassy

A fast and flexible reimplementation of data classes
https://pypi.org/project/dataclassy
Mozilla Public License 2.0
81 stars 9 forks source link

Don't check against isinstance result when checking for hashable properties #18

Closed kgreav closed 3 years ago

kgreav commented 3 years ago

Signed-off-by: Kurt Greaves kurt.greaves@zepben.com

kgreav commented 3 years ago

Was doing some profiling and saw when dealing with sets my code was making a lot of calls to eq. Turns out there were a lot of hash collisions because the hash function wasn't actually using any of the properties when hashing, and the python implementation of set.add resorts to equality checking on everything in the set if you get a hash collision.

I wasn't actually using this hash method, as I'd overridden it using this as a base, but I think it's safe to assume the same problem exists here.

biqqles commented 3 years ago

Good catch, thanks. I should also update the test for hash=True, clearly it was badly designed to let this through. I intend to replace the __hash__ implementation with something much less hacky soon, but this is very useful for now.

kgreav commented 3 years ago

that was the fastest turnaround on a PR for an open source repo i've ever seen, thanks.