Add per-request cache for registry value and proxy lookups #62
Conversation
|
@jensens thanks for creating this Pull Request and helping to improve Plone! TL;DR: Finish pushing changes, pass all other checks, then paste a comment: To ensure that these changes do not break other parts of Plone, the Plone test suite matrix needs to pass, but it takes 30-60 min. Other CI checks are usually much faster and the Plone Jenkins resources are limited, so when done pushing changes and all other checks pass either start all Jenkins PR jobs yourself, or simply add the comment above in this PR to start all the jobs automatically. Happy hacking! |
jensens
force-pushed
the
feat-registry-cache
branch
4 times, most recently
from
4c990e9 to
4628359
Compare
jensens
changed the title
jensens
force-pushed
the
feat-registry-cache
branch
2 times, most recently
from
0ae0c5e to
2ddea2b
Compare
|
@jenkins-plone-org please run jobs |
davisagli
left a comment
davisagli
left a comment
There was a problem hiding this comment.
@jensens This looks interesting! But it broke a few tests in other packages. (Maybe a case where the request is shared between multiple tests without resetting the cache?)
I would put the request in request["_plone_registry_cache"] instead of request._plone_registry_cache. This makes sure it goes in the request.other dict, which is explicitly cleared at the end of processing the request. This is in case there is a reference cycle (maybe some object returned from the registry with an acquisition wrapper that includes a reference to the request?). Maybe I'm being paranoid... I think at least a dict lookup in request.other vs the main request object dict should be pretty equivalent in terms of performance.
jensens
force-pushed
the
feat-registry-cache
branch
from
2ddea2b to
b52402b
Compare
|
@jenkins-plone-org please run jobs |
Cache OOBTree value lookups and forInterface proxy objects on the request to avoid repeated B-tree traversals within a single request.
jensens
force-pushed
the
feat-registry-cache
branch
from
b52402b to
0f2139a
Compare
As suggested by @davisagli in PR review, store the cache in request.other (via item access) rather than on request.__dict__. request.other is explicitly cleared at end-of-request processing, avoiding potential reference cycles and stale caches in tests. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@jenkins-plone-org please run jobs |
zope.publisher's TestRequest does not implement __setitem__, causing TypeError in _get_request_cache(). Gracefully skip caching when the request doesn't support item assignment. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@jenkins-plone-org please run jobs |
3 participants
Summary
__getitem__,get,__contains__) in a per-request dict (request._plone_registry_cache), avoiding repeated OOBTree traversals for the same keys within a single requestforInterface()RecordsProxyobjects per request, keyed by interface identifier + prefix + omit tuplezope.globalrequest.getRequest()(thread-local, ~0.09 µs) to obtain the requestCaching is transparent: no request means no caching, same behavior as before. Cache lives on the request object and is discarded automatically at request end.
RelStorage note
With RelStorage, OOBTree lookups are more expensive due to cache validation overhead and SQL round-trips on cache misses. Multi-process deployments (the main use case for RelStorage) also cause more frequent cache invalidations. The per-request caching bypasses ZODB entirely on repeated lookups, so savings with RelStorage will be larger than with FileStorage.
Benchmark (interleaved, 30 rounds, vanilla Plone 6 site, FileStorage, Python 3.14)
Gains grow with page complexity, number of add-ons, and number of registry lookups per request. These numbers are from a vanilla site with ~3000 registry records; production sites with more add-ons and RelStorage will see larger savings.