Cache provider output per request #94
Conversation
Codecov Report
@@ Coverage Diff @@
## main #94 +/- ##
==========================================
+ Coverage 97.44% 97.52% +0.07%
==========================================
Files 9 9
Lines 627 647 +20
==========================================
+ Hits 611 631 +20
Misses 16 16
|
| @@ -21,11 +22,29 @@ class ZyteApiProvider(PageObjectInputProvider): | |||
|
|
|||
| provided_classes = {BrowserResponse, BrowserHtml, Product} | |||
|
|
|||
| def __init__(self, injector): | |||
| super().__init__(injector) | |||
| self._cached_instances: WeakKeyDictionary[Request, Dict] = WeakKeyDictionary() | |||
There was a problem hiding this comment.
We have a similar solution in ItemProvider. I wonder if it's possible to move this cache to Injector instead, so that there is a single cache which is shared across all providers.
The first step could be using the same cached_istances storage, and keeping all cache handling code as-is. But maybe it'd be possible to improve on this further, e.g. don't pass a class to to_provide if an instance is in the cache, skip a provider if all to_provide classes are in cache, and also automatically add results to the cache.
There was a problem hiding this comment.
I had this in my to-do, but I wonder if we need it now that ItemProvider is gone. I am not sure in which scenario the injector a provider would get the same Request + deps now, given duplicate request filtering.
As a side effect, it allows to work around #91 by requesting indirect outputs directly on the spider callback as well.