From 7508896bfb7ac3cd3ccc81634786fb680aa0c8f2 Mon Sep 17 00:00:00 2001 From: Suraj Shetty <13928957+surajshetty3416@users.noreply.github.com> Date: Wed, 6 Nov 2019 18:46:57 +0530 Subject: [PATCH] perf: Optimise BOM Update Tool (#19236) * perf: Optimize BOM update tool - Remove redundant traverse_tree calls - Cache bom_children data Co-authored-by: Rohit Waghchaure * fix: Replace get_list with db.sql * fix: Enable versioning for updated BOM * fix: Directly save doc_before_save from bom obj instead of using load_doc_before_save * fix: recurssion check performance issue --- erpnext/manufacturing/doctype/bom/bom.py | 32 ++++++++++++++----- .../bom_update_tool/bom_update_tool.py | 30 ++++++++--------- 2 files changed, 39 insertions(+), 23 deletions(-) diff --git a/erpnext/manufacturing/doctype/bom/bom.py b/erpnext/manufacturing/doctype/bom/bom.py index c849f5b7f2..a1bf35fd08 100644 --- a/erpnext/manufacturing/doctype/bom/bom.py +++ b/erpnext/manufacturing/doctype/bom/bom.py @@ -420,8 +420,12 @@ class BOM(WebsiteGenerator): def traverse_tree(self, bom_list=None): def _get_children(bom_no): - return frappe.db.sql_list("""select bom_no from `tabBOM Item` - where parent = %s and ifnull(bom_no, '') != '' and parenttype='BOM'""", bom_no) + children = frappe.cache().hget('bom_children', bom_no) + if children is None: + children = frappe.db.sql_list("""SELECT `bom_no` FROM `tabBOM Item` + WHERE `parent`=%s AND `bom_no`!='' AND `parenttype`='BOM'""", bom_no) + frappe.cache().hset('bom_children', bom_no, children) + return children count = 0 if not bom_list: @@ -534,12 +538,24 @@ class BOM(WebsiteGenerator): def get_child_exploded_items(self, bom_no, stock_qty): """ Add all items from Flat BOM of child BOM""" # Did not use qty_consumed_per_unit in the query, as it leads to rounding loss - child_fb_items = frappe.db.sql("""select bom_item.item_code, bom_item.item_name, - bom_item.description, bom_item.source_warehouse, bom_item.operation, - bom_item.stock_uom, bom_item.stock_qty, bom_item.rate, bom_item.include_item_in_manufacturing, - bom_item.stock_qty / ifnull(bom.quantity, 1) as qty_consumed_per_unit - from `tabBOM Explosion Item` bom_item, tabBOM bom - where bom_item.parent = bom.name and bom.name = %s and bom.docstatus = 1""", bom_no, as_dict = 1) + child_fb_items = frappe.db.sql(""" + SELECT + bom_item.item_code, + bom_item.item_name, + bom_item.description, + bom_item.source_warehouse, + bom_item.operation, + bom_item.stock_uom, + bom_item.stock_qty, + bom_item.rate, + bom_item.include_item_in_manufacturing, + bom_item.stock_qty / ifnull(bom.quantity, 1) AS qty_consumed_per_unit + FROM `tabBOM Explosion Item` bom_item, tabBOM bom + WHERE + bom_item.parent = bom.name + AND bom.name = %s + AND bom.docstatus = 1 + """, bom_no, as_dict = 1) for d in child_fb_items: self.add_to_cur_exploded_items(frappe._dict({ diff --git a/erpnext/manufacturing/doctype/bom_update_tool/bom_update_tool.py b/erpnext/manufacturing/doctype/bom_update_tool/bom_update_tool.py index 87b8f67e53..2ca4d16a07 100644 --- a/erpnext/manufacturing/doctype/bom_update_tool/bom_update_tool.py +++ b/erpnext/manufacturing/doctype/bom_update_tool/bom_update_tool.py @@ -14,23 +14,23 @@ class BOMUpdateTool(Document): def replace_bom(self): self.validate_bom() self.update_new_bom() + frappe.cache().delete_key('bom_children') bom_list = self.get_parent_boms(self.new_bom) updated_bom = [] for bom in bom_list: try: - bom_obj = frappe.get_doc("BOM", bom) - bom_obj.load_doc_before_save() - updated_bom = bom_obj.update_cost_and_exploded_items(updated_bom) + bom_obj = frappe.get_cached_doc('BOM', bom) + # this is only used for versioning and we do not want + # to make separate db calls by using load_doc_before_save + # which proves to be expensive while doing bulk replace + bom_obj._doc_before_save = bom_obj.as_dict() bom_obj.calculate_cost() bom_obj.update_parent_cost() bom_obj.db_update() - if (getattr(bom_obj.meta, 'track_changes', False) and not bom_obj.flags.ignore_version): + if bom_obj.meta.get('track_changes') and not bom_obj.flags.ignore_version: bom_obj.save_version() - - frappe.db.commit() except Exception: - frappe.db.rollback() frappe.log_error(frappe.get_traceback()) def validate_bom(self): @@ -42,22 +42,22 @@ class BOMUpdateTool(Document): frappe.throw(_("The selected BOMs are not for the same item")) def update_new_bom(self): - new_bom_unitcost = frappe.db.sql("""select total_cost/quantity - from `tabBOM` where name = %s""", self.new_bom) + new_bom_unitcost = frappe.db.sql("""SELECT `total_cost`/`quantity` + FROM `tabBOM` WHERE name = %s""", self.new_bom) new_bom_unitcost = flt(new_bom_unitcost[0][0]) if new_bom_unitcost else 0 frappe.db.sql("""update `tabBOM Item` set bom_no=%s, rate=%s, amount=stock_qty*%s where bom_no = %s and docstatus < 2 and parenttype='BOM'""", (self.new_bom, new_bom_unitcost, new_bom_unitcost, self.current_bom)) - def get_parent_boms(self, bom, bom_list=None): - if not bom_list: - bom_list = [] - - data = frappe.db.sql(""" select distinct parent from `tabBOM Item` - where bom_no = %s and docstatus < 2 and parenttype='BOM'""", bom) + def get_parent_boms(self, bom, bom_list=[]): + data = frappe.db.sql("""SELECT DISTINCT parent FROM `tabBOM Item` + WHERE bom_no = %s AND docstatus < 2 AND parenttype='BOM'""", bom) for d in data: + if self.new_bom == d[0]: + frappe.throw(_("BOM recursion: {0} cannot be child of {1}").format(bom, self.new_bom)) + bom_list.append(d[0]) self.get_parent_boms(d[0], bom_list)