perf: cache Meta directly and other improvements (backport frappe#18164) (frappe#18181)

* perf: initialise field map when initialising meta

(cherry picked from commit df8399f)

* perf: simpler, faster meta cache

(cherry picked from commit 6c6a969)

* fix: ensure error is thrown

(cherry picked from commit b529c27)

* refactor: drop _allow_dict support

This is not required anymore since we store full doc anyway. Also since
they share same cache key calling get_cached_value and get_cached_doc
can end up doing duplicate / double work.

(cherry picked from commit abd0187)

* fix: pickle doc objects directly

This seems to be missed out and causes performance regression in IRL
usage when get_doc is called on cached doc already.

(cherry picked from commit bcb5fe9)

* test: basic perf test for rps on get_list

(cherry picked from commit c28ddcc)

* build: pin testing-library/dom temporarily

refer testing-library/dom-testing-library#1169

(cherry picked from commit 40f6a34)

* chore: semantic changes to document caching code

* fix: drop Meta cache during update

Revert "fix: drop Meta cache during update" (frappe#18186)

* Revert "fix: drop Meta cache during update (frappe#18182)"

This reverts commit 656f6df.

* fix: replace meta cache keys

Old keys stored different types of data `dict` changing key to indicate
change in type.

* perf: cache `FormMeta` directly (frappe#18165)

* perf: cache `FormMeta` directly

* perf: check if `dt` is table, use `db.get_value` instead of `get_all`

Co-authored-by: Sagar Vora <[email protected]>
Co-authored-by: Ankush Menat <[email protected]>

Author: 3 people

frappe/__init__.py

@@ -239,7 +239,6 @@ def init(site: str, sites_path: str = ".", new_site: bool = False) -> None:
 	local.jloader = None
 	local.cache = {}
 	local.document_cache = {}
-	local.meta_cache = {}
 	local.form_dict = _dict()
 	local.preload_assets = {"style": [], "script": []}
 	local.session = _dict()
@@ -1065,21 +1064,9 @@ def set_value(doctype, docname, fieldname, value=None):
 	return frappe.client.set_value(doctype, docname, fieldname, value)
 
 
-@overload
-def get_cached_doc(doctype, docname, _allow_dict=True) -> dict:
-	...
-
-
-@overload
 def get_cached_doc(*args, **kwargs) -> "Document":
-	...
-
-
-def get_cached_doc(*args, **kwargs):
-	allow_dict = kwargs.pop("_allow_dict", False)
-
 	def _respond(doc, from_redis=False):
-		if not allow_dict and isinstance(doc, dict):
+		if isinstance(doc, dict):
 			local.document_cache[key] = doc = get_doc(doc)
 
 		elif from_redis:
@@ -1103,6 +1090,12 @@ def _respond(doc, from_redis=False):
 	if not key:
 		key = get_document_cache_key(doc.doctype, doc.name)
 
+	_set_document_in_cache(key, doc)
+
+	return doc
+
+
+def _set_document_in_cache(key: str, doc: "Document") -> None:
 	local.document_cache[key] = doc
 
 	# Avoid setting in local.cache since we're already using local.document_cache above
@@ -1112,8 +1105,6 @@ def _respond(doc, from_redis=False):
 	except Exception:
 		cache().hset("document_cache", key, doc.as_dict(), cache_locally=False)
 
-	return doc
-
 
 def can_cache_doc(args) -> str | None:
 	"""
@@ -1152,7 +1143,7 @@ def get_cached_value(
 	doctype: str, name: str, fieldname: str = "name", as_dict: bool = False
 ) -> Any:
 	try:
-		doc = get_cached_doc(doctype, name, _allow_dict=True)
+		doc = get_cached_doc(doctype, name)
 	except DoesNotExistError:
 		clear_last_message()
 		return
@@ -1188,13 +1179,9 @@ def get_doc(*args, **kwargs) -> "Document":
 
 	doc = frappe.model.document.get_doc(*args, **kwargs)
 
-	# Replace cache
-	if key := can_cache_doc(args):
-		if key in local.document_cache:
-			local.document_cache[key] = doc
-
-		if cache().hexists("document_cache", key):
-			cache().hset("document_cache", key, doc.as_dict())
+	# Replace cache if stale one exists
+	if (key := can_cache_doc(args)) and cache().hexists("document_cache", key):
+		_set_document_in_cache(key, doc)
 
 	return doc
 

frappe/cache_manager.py

@@ -59,8 +59,8 @@
 )
 
 doctype_cache_keys = (
-	"meta",
-	"form_meta",
+	"doctype_meta",
+	"doctype_form_meta",
 	"table_columns",
 	"last_modified",
 	"linked_doctypes",
@@ -117,9 +117,6 @@ def clear_doctype_cache(doctype=None):
 	clear_controller_cache(doctype)
 	cache = frappe.cache()
 
-	if getattr(frappe.local, "meta_cache") and (doctype in frappe.local.meta_cache):
-		del frappe.local.meta_cache[doctype]
-
 	for key in ("is_table", "doctype_modules", "document_cache"):
 		cache.delete_value(key)
 

frappe/commands/utils.py

@@ -881,6 +881,7 @@ def run_ui_tests(
 				"@4tw/cypress-drag-drop@^2",
 				"cypress-real-events",
 				"@testing-library/cypress@^8",
+				"@testing-library/[email protected]",
 				"@cypress/code-coverage@^3",
 			]
 		)

frappe/core/doctype/doctype/doctype.py

@@ -414,10 +414,6 @@ def on_update(self):
 		if not frappe.flags.in_install and hasattr(self, "before_update"):
 			self.sync_global_search()
 
-		# clear from local cache
-		if self.name in frappe.local.meta_cache:
-			del frappe.local.meta_cache[self.name]
-
 		clear_linked_doctype_cache()
 
 	def setup_autoincrement_and_sequence(self):

frappe/desk/form/load.py

@@ -64,11 +64,9 @@ def getdoctype(doctype, with_parent=False, cached_timestamp=None):
 	parent_dt = None
 
 	# with parent (called from report builder)
-	if with_parent:
-		parent_dt = frappe.model.meta.get_parent_dt(doctype)
-		if parent_dt:
-			docs = get_meta_bundle(parent_dt)
-			frappe.response["parent_dt"] = parent_dt
+	if with_parent and (parent_dt := frappe.model.meta.get_parent_dt(doctype)):
+		docs = get_meta_bundle(parent_dt)
+		frappe.response["parent_dt"] = parent_dt
 
 	if not docs:
 		docs = get_meta_bundle(doctype)

frappe/desk/form/meta.py

@@ -35,12 +35,10 @@
 def get_meta(doctype, cached=True):
 	# don't cache for developer mode as js files, templates may be edited
 	if cached and not frappe.conf.developer_mode:
-		meta = frappe.cache().hget("form_meta", doctype)
-		if meta:
-			meta = FormMeta(meta)
-		else:
+		meta = frappe.cache().hget("doctype_form_meta", doctype)
+		if not meta:
 			meta = FormMeta(doctype)
-			frappe.cache().hset("form_meta", doctype, meta.as_dict())
+			frappe.cache().hset("doctype_form_meta", doctype, meta)
 	else:
 		meta = FormMeta(doctype)
 

frappe/model/meta.py

@@ -40,21 +40,31 @@
 from frappe.modules import load_doctype_module
 from frappe.utils import cast, cint, cstr
 
+DEFAULT_FIELD_LABELS = {
+	"name": lambda: _("ID"),
+	"creation": lambda: _("Created On"),
+	"docstatus": lambda: _("Document Status"),
+	"idx": lambda: _("Index"),
+	"modified": lambda: _("Last Updated On"),
+	"modified_by": lambda: _("Last Updated By"),
+	"owner": lambda: _("Created By"),
+	"_user_tags": lambda: _("Tags"),
+	"_liked_by": lambda: _("Liked By"),
+	"_comments": lambda: _("Comments"),
+	"_assign": lambda: _("Assigned To"),
+}
+
 
 def get_meta(doctype, cached=True) -> "Meta":
-	if cached:
-		if not frappe.local.meta_cache.get(doctype):
-			meta = frappe.cache().hget("meta", doctype)
-			if meta:
-				meta = Meta(meta)
-			else:
-				meta = Meta(doctype)
-				frappe.cache().hset("meta", doctype, meta.as_dict())
-			frappe.local.meta_cache[doctype] = meta
+	if not cached:
+		return Meta(doctype)
 
-		return frappe.local.meta_cache[doctype]
-	else:
-		return load_meta(doctype)
+	if meta := frappe.cache().hget("doctype_meta", doctype):
+		return meta
+
+	meta = Meta(doctype)
+	frappe.cache().hset("doctype_meta", doctype, meta)
+	return meta
 
 
 def load_meta(doctype):
@@ -86,32 +96,33 @@ def load_doctype_from_file(doctype):
 class Meta(Document):
 	_metaclass = True
 	default_fields = list(default_fields)[1:]
-	special_doctypes = (
+	special_doctypes = {
 		"DocField",
 		"DocPerm",
 		"DocType",
 		"Module Def",
 		"DocType Action",
 		"DocType Link",
 		"DocType State",
-	)
+	}
 	standard_set_once_fields = [
 		frappe._dict(fieldname="creation", fieldtype="Datetime"),
 		frappe._dict(fieldname="owner", fieldtype="Data"),
 	]
 
 	def __init__(self, doctype):
-		self._fields = {}
+		# from cache
 		if isinstance(doctype, dict):
 			super().__init__(doctype)
+			self.init_field_map()
+			return
 
-		elif isinstance(doctype, Document):
+		if isinstance(doctype, Document):
 			super().__init__(doctype.as_dict())
-			self.process()
-
 		else:
 			super().__init__("DocType", doctype)
-			self.process()
+
+		self.process()
 
 	def load_from_db(self):
 		try:
@@ -126,10 +137,12 @@ def process(self):
 		# don't process for special doctypes
 		# prevent's circular dependency
 		if self.name in self.special_doctypes:
+			self.init_field_map()
 			return
 
 		self.add_custom_fields()
 		self.apply_property_setters()
+		self.init_field_map()
 		self.sort_fields()
 		self.get_valid_columns()
 		self.set_custom_permissions()
@@ -233,36 +246,24 @@ def get_table_field_doctype(self, fieldname):
 
 	def get_field(self, fieldname):
 		"""Return docfield from meta"""
-		if not self._fields:
-			for f in self.get("fields"):
-				self._fields[f.fieldname] = f
 
 		return self._fields.get(fieldname)
 
 	def has_field(self, fieldname):
 		"""Returns True if fieldname exists"""
-		return True if self.get_field(fieldname) else False
+
+		return fieldname in self._fields
 
 	def get_label(self, fieldname):
 		"""Get label of the given fieldname"""
-		df = self.get_field(fieldname)
-		if df:
-			label = df.label
-		else:
-			label = {
-				"name": _("ID"),
-				"creation": _("Created On"),
-				"docstatus": _("Document Status"),
-				"idx": _("Index"),
-				"modified": _("Last Updated On"),
-				"modified_by": _("Last Updated By"),
-				"owner": _("Created By"),
-				"_user_tags": _("Tags"),
-				"_liked_by": _("Liked By"),
-				"_comments": _("Comments"),
-				"_assign": _("Assigned To"),
-			}.get(fieldname) or _("No Label")
-		return label
+
+		if df := self.get_field(fieldname):
+			return df.label
+
+		if fieldname in DEFAULT_FIELD_LABELS:
+			return DEFAULT_FIELD_LABELS[fieldname]()
+
+		return _("No Label")
 
 	def get_options(self, fieldname):
 		return self.get_field(fieldname).options
@@ -273,12 +274,9 @@ def get_link_doctype(self, fieldname):
 		if df.fieldtype == "Link":
 			return df.options
 
-		elif df.fieldtype == "Dynamic Link":
+		if df.fieldtype == "Dynamic Link":
 			return self.get_options(df.options)
 
-		else:
-			return None
-
 	def get_search_fields(self):
 		search_fields = self.search_fields or "name"
 		search_fields = [d.strip() for d in search_fields.split(",")]
@@ -340,20 +338,20 @@ def get_translatable_fields(self):
 
 	def is_translatable(self, fieldname):
 		"""Return true of false given a field"""
-		field = self.get_field(fieldname)
-		return field and field.translatable
+
+		if field := self.get_field(fieldname):
+			return field.translatable
 
 	def get_workflow(self):
 		return get_workflow_name(self.name)
 
 	def get_naming_series_options(self) -> list[str]:
 		"""Get list naming series options."""
 
-		field = self.get_field("naming_series")
-		if field:
+		if field := self.get_field("naming_series"):
 			options = field.options or ""
-
 			return options.split("\n")
+
 		return []
 
 	def add_custom_fields(self):
@@ -450,6 +448,9 @@ def add_custom_links_and_actions(self):
 
 				self.set(fieldname, new_list)
 
+	def init_field_map(self):
+		self._fields = {field.fieldname: field for field in self.fields}
+
 	def sort_fields(self):
 		"""sort on basis of insert_after"""
 		custom_fields = sorted(self.get_custom_fields(), key=lambda df: df.idx)
@@ -666,10 +667,17 @@ def is_single(doctype):
 
 
 def get_parent_dt(dt):
-	parent_dt = frappe.get_all(
-		"DocField", "parent", dict(fieldtype=["in", frappe.model.table_fields], options=dt), limit=1
+	if not frappe.is_table(dt):
+		return ""
+
+	return (
+		frappe.db.get_value(
+			"DocField",
+			{"fieldtype": ("in", frappe.model.table_fields), "options": dt},
+			"parent",
+		)
+		or ""
 	)
-	return parent_dt and parent_dt[0].parent or ""
 
 
 def set_fieldname(field_id, fieldname):