perf: precompute address hash instead of per-instance lru_cache - #1774
Conversation
ZeroconfIPv4Address/ZeroconfIPv6Address assigned self.__hash__ to a functools.cache(lambda ...) in __init__. Because __hash__ sat in __slots__ the memoization did work, but every address object paid to build a full lru_cache wrapper plus a self-capturing lambda at construction time (and a self -> slot -> wrapper -> lambda -> self reference cycle for the GC to reap). Compute the hash once into a plain int slot and expose it through a real __hash__ method. Construction drops ~25-45% in pure-Python micro- benchmarks; hash() stays memoized via the precomputed slot.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1774 +/- ##
=======================================
Coverage 99.77% 99.77%
=======================================
Files 33 33
Lines 3536 3540 +4
Branches 498 498
=======================================
+ Hits 3528 3532 +4
Misses 5 5
Partials 3 3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Merging this PR will improve performance by 48.56%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ⚡ | test_create_ipv4_addresses |
36.5 ms | 23.4 ms | +55.87% |
| ⚡ | test_create_ipv6_addresses |
45.2 ms | 31.9 ms | +41.6% |
Tip
Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.
Comparing bluetoothbot:koan/fix-ipaddress-hash-caching (7f7eea6) with master (bd20c8e)
|
good fix, but can't merge it because we are out of space on pypi. it has to wait a week or so |
What
Replace the per-instance
functools.cache(lambda ...)assigned toself.__hash__inZeroconfIPv4Address/ZeroconfIPv6Addresswith a hash precomputed once into anintslot, exposed through a real__hash__method.Why
Same class of bug as Bluetooth-Devices/cached-ipaddress#108. The old code did:
Because
__hash__was listed in__slots__, the memoization did take effect here (unlike the cached-ipaddress case, where it was dead code). But every address object still paid to allocate a fulllru_cachewrapper and a self-capturing lambda at construction, plus created aself → slot → wrapper → lambda → selfreference cycle the GC has to collect.get_ip_address_object_from_recordbuilds one of these per A/AAAA record on a cache miss, so the construction cost is on the discovery hot path.How
self._hash = IPv4Address.__hash__(self)(resp. IPv6) once in__init__, store in a plainintslot.__hash__method returning it — defined on the type, so dunder lookup resolves correctly and no per-instance wrapper is allocated."__hash__"→"_hash"in__slots__; drop the now-unusedfunctools.cacheimport and thetype: ignore[method-assign].No
cdef classdeclaration for these types inipaddress.pxd, so they stay pure-Python classes under the Cython build;cython -3compiles the module cleanly.Testing
SKIP_CYTHON=1 poetry run pytest tests/utils/test_ipaddress.py tests/services/test_info.py tests/test_listener.py tests/utils/— 85 passed, 1 skipped, 1 xpassed.Quality Report
Changes: 3 files changed, 75 insertions(+), 5 deletions(-)
Code scan: clean
Tests: passed (4 PASSED)
Branch hygiene: clean
Generated by Kōan post-mission quality pipeline