Sales documents is slowly when we put many itens 30+, Precision on JS is the problem

Problem: Sales document is running very slow when it has many items, crashing screen when adding an item or editing quantity/value, freezing time may vary from user hardware.

I have a bad performance runing javascripts, when i do a sales order or sales invoice with many itens.

i have identify a method who make this problem, taxes_and_totals.js, method calculate_item_values.

  • This method is called 4 times when i insert a item on sales invoice.

But the real problem is the method “precision”, this called alot of time, i know precision must be setted…
but if I don’t call and manually set the decimal precision, the runtime goes from 4 seconds to 60 ms

The problem is: precision(field, item)

Anyone has this problem too? my version is v13.13.0

i have made similar experiences, quoatations with lots of items lead to slow behavior. sometimes.

i was not able to pinpoint the actual problem. how did you manage to get down to the individual method? browser dev tools?

First I was commenting code, until I found what was causing the slowness, I found _calculate_rates_and_totals, then I put a runtime calculation.

After that I went back to find out which of the methods that were running in that scope of code that was generating the slowness, I found the function “calculate_item_values”.

So I started trying it out, until I found that overrunning precision(fieldname, doc) is causing this slowness.

Below are all fields that use precision:

I took the test as follows, and it was fast!

This guy is responsible for most of the slowness:
frappe.model.round_floats_in(item);

2 Likes

Very nice analysis. For me the issues are never reproducible enough to go into that direction.
But with your results it motivates me to investigate again.

The round_floats in
frappe/model.js at version-13 · frappe/frappe (github.com)

round_floats_in: function(doc, fieldnames) {
	if(!fieldnames) {
		fieldnames = frappe.meta.get_fieldnames(doc.doctype, doc.parent,
			{"fieldtype": ["in", ["Currency", "Float"]]});
	}
	for(var i=0, j=fieldnames.length; i < j; i++) {
		var fieldname = fieldnames[i];
		doc[fieldname] = flt(doc[fieldname], precision(fieldname, doc));
	}
},

so basically it iterates over all currency and float fields on each item.

one optimization would be to explicitly set the fieldnames, instead of doing it for all currency and floats…

and round_floats_in is called from multiple controllers within erpnext:
Search · round_floats_in (github.com)

I believe that the method “frappe.model.round_floats_in” should receive the fields that must be updated, but it is a little complex to know what will impact passing the specific fields.

1 Like

with the original code, in a situation with 49 items, adding the 50 item, it takes 5.2 seconds to add the new item.

Passing the specific fields takes 2.3 seconds.

However, this is still not a good result, considering that my computer is very good, as javascript runs on the client, if his computer is bad, it will be slower…

1 Like

could it be that also set_currency_label in transaction.js is slow?

I don’t think so, because it’s not running in a loop.

The problem with round_floats_in is that it executes all quantity and money fields item by item. besides calling the calculate_item_values method 4 times. which makes the whole process even slower.

the first problem is: why calculate_taxes_and_totals is called 4 times when i insert a new item…

on transaction.js i know where the “calculate_taxes_and_totals” is called.

1 - on change value from field “rate” event

2 - on price_list_rate function, dont make sense price_list_rate call “calculate_taxes_and_totals”, because on rate change value “calculate_taxes_and_totals” will be called…

3 - On change value from field “rate” event Again

4 - on shipping_rule function, but idk if need to be called here, when validate the form all value will be recalculed.

This function is called every time when i insert a item…

Good analysis.

This is a known problem .

What could be the solution?

I optimized the execution time of the method “calculate_item_values”, in my situation it went from 1300ms to 320ms.

1 - I created a new method “round_floats_childs_in”, it has the same purpose as “round_floats_in”, but it aims to get precision() only once for each field in the first position of the loop, then for the other positions of the for use this value of the local variable.

2 - Now I make the round_floats_childs_in call, doing what should be done for all items, outside the for.

I will learn to contribute! i never did it…

2 Likes

I optimized the execution time of the method “calculate_item_values”, again, now from 320ms to 66.6 ms.

3 Likes

that sounds really nice and impressive! i will try to recreate your improvements in my environment in the next few days.
do you have the code already checked in somewhere?

no, i will learn how to contribute yet

2 Likes

looking forward your first contribution.

Hi @PHF

Great work. Looking forward to your contribution :+1:t3:

Cheers !

https://github.com/frappe/erpnext/pull/28332
https://github.com/frappe/frappe/pull/14941

3 Likes

Good work and great analysis!:grin: