perf: speedup @redis_cache (#28813)

* perf: reduce one redis call for redis_cache

* fix: Always set value in local even if it expires

frappe/tests/test_caching.py

@@ -110,6 +110,7 @@ class TestRedisCache(FrappeAPITestCase):
 		self.assertEqual(function_call_count, 1)
 
 		time.sleep(CACHE_TTL * 1.5)
+		frappe.local.cache.clear()
 		self.assertEqual(calculate_area(10), 314)
 		self.assertEqual(function_call_count, 2)
 

frappe/tests/test_perf.py

@@ -32,6 +32,7 @@ from frappe.tests import IntegrationTestCase
 from frappe.tests.test_api import FrappeAPITestCase
 from frappe.tests.test_query_builder import run_only_if
 from frappe.utils import cint
+from frappe.utils.caching import redis_cache
 from frappe.website.path_resolver import PathResolver
 
 TEST_USER = "test@example.com"
@@ -201,9 +202,26 @@ class TestPerformance(IntegrationTestCase):
 
 	def test_get_doc_cache_calls(self):
 		frappe.get_doc("User", "Administrator")
-		with self.assertRedisCallCounts(1):
+		with self.assertRedisCallCounts(0):
 			frappe.get_doc("User", "Administrator")
 
+	def test_local_caching(self):
+		frappe.get_cached_doc("User", "Administrator")
+		with self.assertRedisCallCounts(0):
+			frappe.get_cached_doc("User", "Administrator")
+
+	def test_redis_cache_calls(self):
+		redis_cached_func()  # warmup
+
+		# Repeat call should use locally cached value
+		with self.assertRedisCallCounts(0):
+			redis_cached_func()
+
+		frappe.local.cache.clear()
+		# Without local cache - only one call required
+		with self.assertRedisCallCounts(1):
+			redis_cached_func()
+
 	def test_one_time_setup(self):
 		site = frappe.local.site
 		frappe.init(site, force=True)
@@ -245,3 +263,8 @@ class TestOverheadCalls(FrappeAPITestCase):
 		self.get(self.resource("User", "Administrator"), {"sid": sid})
 		with self.assertRedisCallCounts(19), self.assertQueryCount(self.BASE_SQL_CALLS + 1 + tables):
 			self.get(self.resource("User", "Administrator"), {"sid": sid})
+
+
+@redis_cache
+def redis_cached_func():
+	return 42

frappe/utils/caching.py

@@ -164,8 +164,15 @@ def redis_cache(ttl: int | None = 3600, user: str | bool | None = None, shared:
 		@wraps(func)
 		def redis_cache_wrapper(*args, **kwargs):
 			func_call_key = func_key + "::" + str(__generate_request_cache_key(args, kwargs))
+			cached_val = frappe.cache.get_value(func_call_key, user=user, shared=shared)
+			if cached_val is not None:
+				return cached_val
+
+			# Edge Case: None can mean two things: cache miss or the result itself is `None`
+			# RedisWrapper doesn't give us any way to handle this cleanly.
 			if frappe.cache.exists(func_call_key, user=user, shared=shared):
-				return frappe.cache.get_value(func_call_key, user=user, shared=shared)
+				return None
+
 			val = func(*args, **kwargs)
 			ttl = getattr(func, "ttl", 3600)
 			frappe.cache.set_value(func_call_key, val, expires_in_sec=ttl, user=user, shared=shared)

frappe/utils/redis_wrapper.py

@@ -60,7 +60,6 @@ class RedisWrapper(redis.Redis):
 		"""
 		key = self.make_key(key, user, shared)
 
-		if not expires_in_sec:
 		frappe.local.cache[key] = val
 
 		with suppress(redis.exceptions.ConnectionError):