Minor optimisation to cachedListDescriptor

we can avoid re-using `missing`, which saves looking up entries in
`deferreds_map`, and means we don't need to copy it.

Author: richvdh

synapse/util/caches/descriptors.py

@@ -346,10 +346,10 @@ class DeferredCacheListDescriptor(_CacheDescriptorBase):
     """Wraps an existing cache to support bulk fetching of keys.
 
     Given an iterable of keys it looks in the cache to find any hits, then passes
-    the tuple of missing keys to the wrapped function.
+    the set of missing keys to the wrapped function.
 
-    Once wrapped, the function returns a Deferred which resolves to the list
-    of results.
+    Once wrapped, the function returns a Deferred which resolves to a Dict mapping from
+    input key to output value.
     """
 
     def __init__(
@@ -452,14 +452,14 @@ def complete_all(res: Dict[Hashable, Any]) -> None:
                     # the wrapped function has completed. It returns a dict.
                     # We can now update our own result map, and then resolve the
                     # observable deferreds in the cache.
-                    for e in missing:
+                    for e, d1 in deferreds_map.items():
                         val = res.get(e, None)
                         # make sure we update the results map before running the
                         # deferreds, because as soon as we run the last deferred, the
                         # gatherResults() below will complete and return the result
                         # dict to our caller.
                         results[e] = val
-                        deferreds_map[e].callback(val)
+                        d1.callback(val)
 
                 def errback_all(f: Failure) -> None:
                     # the wrapped function has failed. Propagate the failure into
@@ -470,9 +470,7 @@ def errback_all(f: Failure) -> None:
                         d1.errback(f)
 
                 args_to_call = dict(arg_dict)
-                # copy the missing set before sending it to the callee, to guard against
-                # modification.
-                args_to_call[self.list_name] = tuple(missing)
+                args_to_call[self.list_name] = missing
 
                 # dispatch the call, and attach the two handlers
                 defer.maybeDeferred(

tests/util/caches/test_descriptors.py

@@ -673,14 +673,14 @@ async def list_fn(self, args1, arg2):
             self.assertEqual(current_context(), SENTINEL_CONTEXT)
             r = yield d1
             self.assertEqual(current_context(), c1)
-            obj.mock.assert_called_once_with((10, 20), 2)
+            obj.mock.assert_called_once_with({10, 20}, 2)
             self.assertEqual(r, {10: "fish", 20: "chips"})
             obj.mock.reset_mock()
 
             # a call with different params should call the mock again
             obj.mock.return_value = {30: "peas"}
             r = yield obj.list_fn([20, 30], 2)
-            obj.mock.assert_called_once_with((30,), 2)
+            obj.mock.assert_called_once_with({30}, 2)
             self.assertEqual(r, {20: "chips", 30: "peas"})
             obj.mock.reset_mock()
 
@@ -701,7 +701,7 @@ async def list_fn(self, args1, arg2):
             obj.mock.return_value = {40: "gravy"}
             iterable = (x for x in [10, 40, 40])
             r = yield obj.list_fn(iterable, 2)
-            obj.mock.assert_called_once_with((40,), 2)
+            obj.mock.assert_called_once_with({40}, 2)
             self.assertEqual(r, {10: "fish", 40: "gravy"})
 
     def test_concurrent_lookups(self):
@@ -729,7 +729,7 @@ def list_fn(self, args1) -> "Deferred[dict]":
         d3 = obj.list_fn([10])
 
         # the mock should have been called exactly once
-        obj.mock.assert_called_once_with((10,))
+        obj.mock.assert_called_once_with({10})
         obj.mock.reset_mock()
 
         # ... and none of the calls should yet be complete
@@ -771,7 +771,7 @@ async def list_fn(self, args1, arg2):
         # cache miss
         obj.mock.return_value = {10: "fish", 20: "chips"}
         r1 = yield obj.list_fn([10, 20], 2, on_invalidate=invalidate0)
-        obj.mock.assert_called_once_with((10, 20), 2)
+        obj.mock.assert_called_once_with({10, 20}, 2)
         self.assertEqual(r1, {10: "fish", 20: "chips"})
         obj.mock.reset_mock()