From 3675f3c797f62cca7111998e6983d80a93775026 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Tue, 17 Dec 2024 11:23:33 +0530 Subject: [PATCH 1/4] perf(site_cache): reduce access to frappe.local namespace This change also allows calling @site_cache during init, as long as `site` parameter is set. --- frappe/utils/caching.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/frappe/utils/caching.py b/frappe/utils/caching.py index 2299c95236..3076492bf1 100644 --- a/frappe/utils/caching.py +++ b/frappe/utils/caching.py @@ -122,7 +122,7 @@ def site_cache(ttl: int | None = None, maxsize: int | None = None) -> Callable: @wraps(func) def site_cache_wrapper(*args, **kwargs): - if getattr(frappe.local, "initialised", None): + if site := getattr(frappe.local, "site", None): func_call_key = json.dumps((args, kwargs)) if hasattr(func, "ttl") and datetime.datetime.now(datetime.timezone.utc) >= func.expiration: @@ -131,15 +131,13 @@ def site_cache(ttl: int | None = None, maxsize: int | None = None) -> Callable: seconds=func.ttl ) - if hasattr(func, "maxsize") and len(_SITE_CACHE[func_key][frappe.local.site]) >= func.maxsize: - _SITE_CACHE[func_key][frappe.local.site].pop( - next(iter(_SITE_CACHE[func_key][frappe.local.site])), None - ) + if hasattr(func, "maxsize") and len(_SITE_CACHE[func_key][site]) >= func.maxsize: + _SITE_CACHE[func_key][site].pop(next(iter(_SITE_CACHE[func_key][site])), None) - if func_call_key not in _SITE_CACHE[func_key][frappe.local.site]: - _SITE_CACHE[func_key][frappe.local.site][func_call_key] = func(*args, **kwargs) + if func_call_key not in _SITE_CACHE[func_key][site]: + _SITE_CACHE[func_key][site][func_call_key] = func(*args, **kwargs) - return _SITE_CACHE[func_key][frappe.local.site][func_call_key] + return _SITE_CACHE[func_key][site][func_call_key] return func(*args, **kwargs) From ad0e8ed73568cd40c149decd6580177aec24b78c Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Tue, 17 Dec 2024 11:33:21 +0530 Subject: [PATCH 2/4] test: frappe.init patching --- frappe/tests/test_perf.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/frappe/tests/test_perf.py b/frappe/tests/test_perf.py index 66db3eeb37..d441ac65ce 100644 --- a/frappe/tests/test_perf.py +++ b/frappe/tests/test_perf.py @@ -204,6 +204,16 @@ class TestPerformance(IntegrationTestCase): with self.assertRedisCallCounts(1): frappe.get_doc("User", "Administrator") + def test_one_time_setup(self): + site = frappe.local.site + frappe.init(site, force=True) + run = frappe.qb._BuilderClasss.run + + frappe.init(site, force=True) + patched_run = frappe.qb._BuilderClasss.run + + self.assertIs(run, patched_run, "frappe.init should run one-time patching code just once") + @run_only_if(db_type_is.MARIADB) class TestOverheadCalls(FrappeAPITestCase): From f9ed28956dda59afac3d3ef5030959cccb4ad46b Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Tue, 17 Dec 2024 11:41:55 +0530 Subject: [PATCH 3/4] perf: use monotonic time instead of realtime for eviction datetime is complex, slow and not really required for this use case. --- frappe/utils/caching.py | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/frappe/utils/caching.py b/frappe/utils/caching.py index 3076492bf1..50b9edabb9 100644 --- a/frappe/utils/caching.py +++ b/frappe/utils/caching.py @@ -1,8 +1,8 @@ # Copyright (c) 2022, Frappe Technologies Pvt. Ltd. and Contributors # License: MIT. Check LICENSE -import datetime import json +import time from collections import defaultdict from collections.abc import Callable from functools import wraps @@ -113,9 +113,7 @@ def site_cache(ttl: int | None = None, maxsize: int | None = None) -> Callable: if ttl is not None and not callable(ttl): func.ttl = ttl - func.expiration = datetime.datetime.now(datetime.timezone.utc) + datetime.timedelta( - seconds=func.ttl - ) + func.expiration = time.monotonic() + func.ttl if maxsize is not None and not callable(maxsize): func.maxsize = maxsize @@ -125,13 +123,12 @@ def site_cache(ttl: int | None = None, maxsize: int | None = None) -> Callable: if site := getattr(frappe.local, "site", None): func_call_key = json.dumps((args, kwargs)) - if hasattr(func, "ttl") and datetime.datetime.now(datetime.timezone.utc) >= func.expiration: + if hasattr(func, "ttl") and time.monotonic() >= func.expiration: func.clear_cache() - func.expiration = datetime.datetime.now(datetime.timezone.utc) + datetime.timedelta( - seconds=func.ttl - ) + func.expiration = time.monotonic() + func.ttl if hasattr(func, "maxsize") and len(_SITE_CACHE[func_key][site]) >= func.maxsize: + # Note: This implements FIFO eviction policty _SITE_CACHE[func_key][site].pop(next(iter(_SITE_CACHE[func_key][site])), None) if func_call_key not in _SITE_CACHE[func_key][site]: From 9951f151f53e8f95e45a931696b69fe6d5b3741d Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Tue, 17 Dec 2024 11:44:22 +0530 Subject: [PATCH 4/4] perf!: Drop support for unhashable arguments Just like LRU cache, no need to support unhashable types in site_cache. Current usage in codebase also shows that it's not required and json.dumps is quite slow. --- frappe/utils/caching.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/frappe/utils/caching.py b/frappe/utils/caching.py index 50b9edabb9..13897fe49a 100644 --- a/frappe/utils/caching.py +++ b/frappe/utils/caching.py @@ -1,7 +1,6 @@ # Copyright (c) 2022, Frappe Technologies Pvt. Ltd. and Contributors # License: MIT. Check LICENSE -import json import time from collections import defaultdict from collections.abc import Callable @@ -121,7 +120,7 @@ def site_cache(ttl: int | None = None, maxsize: int | None = None) -> Callable: @wraps(func) def site_cache_wrapper(*args, **kwargs): if site := getattr(frappe.local, "site", None): - func_call_key = json.dumps((args, kwargs)) + func_call_key = __generate_request_cache_key(args, kwargs) if hasattr(func, "ttl") and time.monotonic() >= func.expiration: func.clear_cache()