[Performance] Reducing number of DB calls

[Sachin_Mane]

Continuing the discussion from [Performance] 1 Sales Invoice Save makes ~700+ DB calls. ~1000+ DB calls on Submit:

@szufisher db_insert_multi is a good idea. We just need to see if it fits well in the meta data driven framework and make it generic enough to work with all doctypes having child tables. Worth giving a try.
Some child tables like Quotation Item might need tweaking as currently they follow a naming series which will be a challenge for multi insert.

@rmehta, @nabinhait what do you think?

[szufisher]

@Sachin_Mane,
Other 2 improvement areas

1. Batch creating stock ledger and general ledger entries instead of one by one

Currently there is only one table for stock ledger entry and general ledger(gl) entry respectively, in other word not like transactions split by master and child tables such as Purchase Order as master, Purchase Order Item as child, normally when auto creating stock ledger and general ledger entries for one original transaction, there are many sle and gle entries to be created, at the moment the sle and gle entries are created one by one, which resulted in many DB calls, for original transactions associated with a lot of items( a lot of gl accounts), same as the proposed db_insert_multi for child table, there can be insert_multi method to handle creating(inserting) multi docs, for the most frequent call to auto create sle and gle entries, it will be a big performance improvement

2. Fetch multiple auto names from naming series

At the moment for document with auto name by naming series, even when creating multiple docs for the same transaction , e.g stock ledger entry and general ledger entry, getseries( fetch the auto name) is handled one by one, which resulted in a lot of unnecessary DB calls to the Series table. instead of calling getseries to get one auto name a time, the getseries method can be adapted to accept an input parameter to allow getting multiple auto names, the calling function is to assign internally the returned names accordingly, thus the DB calls to the Series table can be reduced to only 2, one for select the current number, the other is to update the new current number as old number + requested qty.

def getseries(key, digits, doctype=‘’):
# series created ?
current = frappe.db.sql(“select current from tabSeries where name=%s for update”, (key,))
if current and current[0][0] is not None:
current = current[0][0]
# yes, update it
frappe.db.sql(“update tabSeries set current = current+1 where name=%s”, (key,))
current = cint(current) + 1
else:
# no, create it
frappe.db.sql(“insert into tabSeries (name, current) values (%s, 1)”, (key,))
current = 1
return (‘%0’+str(digits)+‘d’) % current

Another idea of the auto name for stock ledger and general ledger entry is to use per transaction document number plus sequence number as name. currently a new name is assigned to each record, e.g GLE000001, GLE000002,… actually these 2 gle entries linked to the same original business transaction, logically all the gle entries created by the original transaction are to be assigned same document number(in other ERP system), per the above analysis, In ERPNext there is no separation between master and child table for gle, in order to reduce the DB calls to Series table and also bind one transaction together/assign same document number for one transaction, I would like to propose the sle name to be defined as combination of document number and item/sequence no, e.g as GLE000001-01, GLE000001-02, we only need to get one auto name for a single transaction, also from the name we can easily see which gle entries are actually of the same financial document, auto created by the same original business transaction.

[szufisher]

the locally tested version of the insert_multi(model/document.py) is as below

def insert_multi(self, ignore_permissions=None, ignore_if_duplicate=False, ignore_mandatory=None, docs=None):
	"""Insert the document in the database (as a new document).
	This will check for user permissions and execute `before_insert`,
	`validate`, `on_update`, `after_insert` methods if they are written.

	:param ignore_permissions: Do not check permissions if True."""
	if self.flags.in_print or not docs:
		return

	for doc in docs:
		doc.flags.email_alerts_executed = []
		doc.flags.custom_server_action_executed = []

		if ignore_permissions != None:
			doc.flags.ignore_permissions = ignore_permissions

		if ignore_mandatory != None:
			doc.flags.ignore_mandatory = ignore_mandatory

		doc.set("__islocal", True)

		doc.check_permission("create")
		doc._set_defaults()
		doc.set_user_and_timestamp()
		doc.set_docstatus()
		doc.check_if_latest()
		doc.run_method("before_insert")
		doc.set_new_name()
		doc.set_parent_in_children()
		doc.validate_higher_perm_levels()

		doc.flags.in_insert = True
		doc._validate_links()
		doc.run_before_save_methods()
		doc._validate()
		doc.set_docstatus()
		doc.flags.in_insert = False

	try:
		doc.db_insert_multi(docs)
	except frappe.DuplicateEntryError as e:
		if not ignore_if_duplicate:
			raise e

	for doc in docs:
		doc.db_insert_multi(doc.get_all_children())

		doc.run_method("after_insert")
		doc.flags.in_insert = True

		if doc.get("amended_from"):
			doc.copy_attachments_from_amended_from()

		doc.run_post_save_methods()
		doc.flags.in_insert = False

		# delete __islocal
		if hasattr(doc, "__islocal"):
			delattr(doc, "__islocal")

	return True

the revised save_entries (erpnext/erpnext/accounts/general_ledger.py) is as below

def save_entries(gl_map, adv_adj, update_outstanding, from_repost=False):
if not from_repost:
	validate_account_for_perpetual_inventory(gl_map)		
round_off_debit_credit(gl_map)
gle_list = []
for entry in gl_map:
	entry.update({"doctype": "GL Entry"})
	gle = frappe.get_doc(entry)
	gle.flags.ignore_permissions = 1
	gle.flags.from_repost = from_repost
	gle_list.append(gle)
gle_list[0].insert_multi(docs=gle_list)
for gle in gle_list:
	gle.run_method("on_update_with_args", adv_adj, update_outstanding, from_repost)
	gle.submit()
	#make_entry(entry, adv_adj, update_outstanding, from_repost)
# check against budget
if not from_repost:
	for entry in gl_map:
		validate_expense_against_budget(entry)

[szufisher]

@Sachin_Mane
have you tried the db_insert_multi and insert_multi, per testing on my local instance it works as expected for the following transactions
-Purchase Order
-Purchase Receipt
-Purchase Invoice
-Sales Quotation
-Sales Order
-Sales Delivery
-Sales Invoice
-Journal Entry

I would like to get enough feedback from the community in the forum before actually start a new pull request.

[rmehta]

I would strongly recommend tests and benchmarks before doing the exercise.

[szufisher]

anyone who can help to do the test on his/her own local instance?

[rmehta]

Just compare performance on a large number of invoices with and without your script and share some results.

[ahashans]

@szufisher how can I use this insert_multi() method in sales quotation submit operation? I mean, where is the method like save_items() method for sales quotation?

[szufisher]

in you local ERPNext instance, goto the erpnext/selling/doctype/quotation/quaotation.py file

add the following three methods into the quotation class
def db_insert_multi(self, docs):
“”“INSERT multi documents (with valid columns) in the database.”“”
def insert_multi():
try:
columns = doc_dict_for_insert[0].keys()
sql = “”“insert into tab{doctype}
({columns}) values “””.format(
doctype=doctype,
columns=“, “.join([”" + c + "” for c in columns]))
placeholders = ‘’
values =[]
for d in doc_dict_for_insert:
rec_placeholder = ‘(%s)’ %(“, “.join([”%s”] * len(columns)))
if placeholders :
placeholders = ‘%s,%s’ %(placeholders , rec_placeholder)
else:
placeholders = rec_placeholder
values.extend(list(d.values()))
sql = ‘%s %s’ %(sql, placeholders )
frappe.db.sql(sql, values)
except Exception as e:
raise
for doc in doc_obj_for_insert:
doc.set(“__islocal”, False)
if not docs:
return
doctype = docs[0].doctype
doc_dict_for_insert = []
doc_obj_for_insert = []
count = 0
for doc in docs:
count += 1
if not doc.name:
# name will be set by document class in most cases
set_new_name(doc)
if not doc.creation:
doc.creation = doc.modified = now()
doc.created_by = doc.modifield_by = frappe.session.user

	d = self.get_valid_dict(doc=doc)
	if doc.doctype != doctype or count > 500:
		insert_multi()
		doc_dict_for_insert = []
		doc_obj_for_insert = []
		count = 0
	doctype= doc.doctype
	doc_dict_for_insert.append(d)
	doc_obj_for_insert.append(doc)
insert_multi()

def get_valid_dict(self, sanitize=True, convert_dates_to_str=False, doc = None):
doc = doc or self
d = frappe._dict()
for fieldname in doc.meta.get_valid_columns():
d[fieldname] = doc.get(fieldname)

	# if no need for sanitization and value is None, continue
	if not sanitize and d[fieldname] is None:
		continue

	df = doc.meta.get_field(fieldname)
	if df:
		if df.fieldtype=="Check":
			if d[fieldname]==None:
				d[fieldname] = 0

			elif (not isinstance(d[fieldname], int) or d[fieldname] > 1):
				d[fieldname] = 1 if cint(d[fieldname]) else 0

		elif df.fieldtype=="Int" and not isinstance(d[fieldname], int):
			d[fieldname] = cint(d[fieldname])

		elif df.fieldtype in ("Currency", "Float", "Percent") and not isinstance(d[fieldname], float):
			d[fieldname] = flt(d[fieldname])

		elif df.fieldtype in ("Datetime", "Date", "Time") and d[fieldname]=="":
			d[fieldname] = None

		elif df.get("unique") and cstr(d[fieldname]).strip()=="":
			# unique empty field should be set to None
			d[fieldname] = None

		if isinstance(d[fieldname], list) and df.fieldtype != 'Table':
			frappe.throw(_('Value for {0} cannot be a list').format(_(df.label)))

		if convert_dates_to_str and isinstance(d[fieldname], (datetime.datetime, datetime.time, datetime.timedelta)):
			d[fieldname] = str(d[fieldname])

return d

def insert(self, ignore_permissions=None, ignore_links=None, ignore_if_duplicate=False, ignore_mandatory=None):

# children
#for d in self.get_all_children():
#	d.db_insert()

self.db_insert_multi(self.get_all_children())

let me know your findings.

[ahashans]

@szufisher can you please reindent the code ? I am having trouble with the indentation.

[szufisher]

kindly refer to this file erpnext/quotation.py at develop · szufisher/erpnext · GitHub,
please note the following lines are newly added
lines 9~11
lines 29~191

[Sachin_Mane]

@szufisher thanks,

We will try this and let you know our findings.