Raymond Hettinger <[email protected]> added the comment:
AFAICT, the only capability added by the PR is keeping a weak reference to the
instance.
This doesn't improve the hit rate and will make the hit rate worse for
instances that define __hash__. In the PR's example, two vector instances with
equal coordinates would not be recognized as being equivalent. Note,
dataclasses make it trivially easy to add custom __eq__ and __hash__ support.
Users also lose the ability to limit maximum memory utilization. Managing the
combined size of many caches, one per instance, is much more difficult that
setting an automatic maxsize limit on a single lru_cache.
Users further lose the ability to clear the entire cache all at once. This can
be important in testing.
AFAICT, the only benefit is that short-lived instances will be freed earlier
than if they waited to age out of an lru_cache. This would only ever matter if
the instances were large, short-lived, and would never have other equivalent
instances that could create a cache hit.
Lastly, I don't like the example given in the docs for the PR. We really want
the class to define __hash__ and __eq__ methods. This doesn't just improve the
hit rate, it is also necessary for avoiding bugs. If the coordinates gets
mutate, the cache has no way of knowing that its entry is invalid:
v = Vector(10, 20, 30)
print(v.norm())
v.coordinates = (11, 22, 33)
print(v.norm())
----------
nosy: +rhettinger
_______________________________________
Python tracker <[email protected]>
<https://bugs.python.org/issue45588>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe:
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
