From fab464effb8061a8f0c91e4b84e30c4c230795be Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Tue, 14 Jan 2025 13:40:39 +0530 Subject: [PATCH 1/3] feat: support shared keys in client cache --- frappe/tests/test_client_cache.py | 6 ++++++ frappe/utils/redis_wrapper.py | 14 +++++++------- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/frappe/tests/test_client_cache.py b/frappe/tests/test_client_cache.py index 549d015d3c..0fdb6d7c0e 100644 --- a/frappe/tests/test_client_cache.py +++ b/frappe/tests/test_client_cache.py @@ -97,3 +97,9 @@ class TestClientCache(IntegrationTestCase): frappe.client_cache.set_value(TEST_KEY, val) self.assertEqual(frappe.client_cache.get_value(TEST_KEY), frappe.cache.get_value(TEST_KEY)) + + def test_shared_keys(self): + val = frappe.generate_hash() + frappe.client_cache.set_value(TEST_KEY, val, shared=True) + with self.assertRedisCallCounts(0): + self.assertEqual(frappe.client_cache.get_value(TEST_KEY, shared=True), val) diff --git a/frappe/utils/redis_wrapper.py b/frappe/utils/redis_wrapper.py index ab9fdde215..3474d0c25c 100644 --- a/frappe/utils/redis_wrapper.py +++ b/frappe/utils/redis_wrapper.py @@ -482,11 +482,11 @@ class ClientCache: ) self.invalidator_thread = self.run_invalidator_thread() - def get_value(self, key): + def get_value(self, key, *, shared=False): if not self.healthy: - return self.redis.get_value(key) + return self.redis.get_value(key, shared=shared) - key = self.redis.make_key(key) + key = self.redis.make_key(key, shared=shared) try: val = self.cache[key] if time.monotonic() < val.expiry and self.healthy: @@ -518,8 +518,8 @@ class ClientCache: return val - def set_value(self, key, val): - key = self.redis.make_key(key) + def set_value(self, key, val, *, shared=False): + key = self.redis.make_key(key, shared=shared) self.ensure_max_size() self.redis.set_value(key, val, shared=True) with self.lock: @@ -536,8 +536,8 @@ class ClientCache: with self.lock, suppress(RuntimeError): self.cache.pop(next(iter(self.cache)), None) - def delete_value(self, key): - key = self.redis.make_key(key) + def delete_value(self, key, *, shared=False): + key = self.redis.make_key(key, shared=shared) self.redis.delete_value(key, shared=True) with self.lock: self.cache.pop(key, None) From 4d1ef02e2563ab72d3abc90d5f139659f0a0c173 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Tue, 14 Jan 2025 13:52:06 +0530 Subject: [PATCH 2/3] feat: generator support for client cache Similar to redis, for ergonomics --- frappe/tests/test_client_cache.py | 8 ++++++++ frappe/utils/redis_wrapper.py | 6 +++--- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/frappe/tests/test_client_cache.py b/frappe/tests/test_client_cache.py index 0fdb6d7c0e..f9818b2a72 100644 --- a/frappe/tests/test_client_cache.py +++ b/frappe/tests/test_client_cache.py @@ -103,3 +103,11 @@ class TestClientCache(IntegrationTestCase): frappe.client_cache.set_value(TEST_KEY, val, shared=True) with self.assertRedisCallCounts(0): self.assertEqual(frappe.client_cache.get_value(TEST_KEY, shared=True), val) + + def test_generator(self): + val = frappe.generate_hash() + with self.assertRedisCallCounts(2, exact=True): + self.assertEqual(frappe.client_cache.get_value(TEST_KEY, generator=lambda: val), val) + + with self.assertRedisCallCounts(0): + self.assertEqual(frappe.client_cache.get_value(TEST_KEY, generator=lambda: val), val) diff --git a/frappe/utils/redis_wrapper.py b/frappe/utils/redis_wrapper.py index 3474d0c25c..e6b37b288f 100644 --- a/frappe/utils/redis_wrapper.py +++ b/frappe/utils/redis_wrapper.py @@ -482,9 +482,9 @@ class ClientCache: ) self.invalidator_thread = self.run_invalidator_thread() - def get_value(self, key, *, shared=False): + def get_value(self, key, *, shared=False, generator=None): if not self.healthy: - return self.redis.get_value(key, shared=shared) + return self.redis.get_value(key, shared=shared, generator=generator) key = self.redis.make_key(key, shared=shared) try: @@ -501,7 +501,7 @@ class ClientCache: with self.lock: self.cache[key] = _PLACEHOLDER_VALUE - val = self.redis.get_value(key, shared=True, use_local_cache=not self.healthy) + val = self.redis.get_value(key, shared=True, use_local_cache=not self.healthy, generator=generator) # Note: We should not "cache" the cache-misses in client cache. # This cache is long lived and "misses" are not tracked by redis so they'll never get From cde16627919b572c80598b20faab35194512ce10 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Tue, 14 Jan 2025 13:57:41 +0530 Subject: [PATCH 3/3] perf: use client side cache for assets.json Single shared key, worth the costs. --- frappe/utils/__init__.py | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/frappe/utils/__init__.py b/frappe/utils/__init__.py index a8aba487e2..d10a1c43ed 100644 --- a/frappe/utils/__init__.py +++ b/frappe/utils/__init__.py @@ -961,18 +961,15 @@ def get_assets_json(): return assets - if not hasattr(frappe.local, "assets_json"): - if not frappe.conf.developer_mode: - frappe.local.assets_json = frappe.cache.get_value( - "assets_json", - _get_assets, - shared=True, - ) + if not frappe.conf.developer_mode: + return frappe.client_cache.get_value( + "assets_json", + shared=True, + generator=_get_assets, + ) - else: - frappe.local.assets_json = _get_assets() - - return frappe.local.assets_json + else: + return _get_assets() def get_bench_relative_path(file_path):