Details
-
Type:
Task
-
Status:
Closed
-
Priority:
Blocker
-
Resolution: Fixed
-
Affects Version/s: 2.3
-
Component/s: User Interface
-
Security Level: Public (Public: Anyone can view)
-
Labels:None
-
Similar issues:
KULRICE-8324 Performance improvements for totalling and group totalling KULRICE-10093 Validation framework performance problem with writeMessagesForPage recursion KULRICE-11243Fill AFT Per-Screen Item Gap: KRAD Library: Column Calculations KULRICE-9915Add line client Javascript runs for over a minute KULRICE-12476KRAD performance issues with OLE KULRICE-11864Column calculation fixes - style, duplicate total elements, readOnly totaling KULRICE-8329Optimize performance of collection totaling and grouping KULRICE-6835Issues with client side validation KULRICE-13524 Rice Performance Client App Double Login -
Rice Module:KRAD
-
Application Requirement:KS
-
KAI Review Status:Not Required
-
KTI Review Status:Not Required
-
Code Review Status:Not Required
-
Include in Release Notes?:Yes
Description
Performance issue caused by too many recursions:
Email from Dan:
Perhaps the handleMessagesAtGroup and writeMessagesForGroup could return the data containing the calculations and sum the counts as the tree is traversed.
If you are traversing from a node up through its parent, the parent’s count could be updated with the difference in the child’s count until you reach the root node. That way you don’t need to call recursiveGroupMessageCount, you just need to update from the change in error count in handleMessagesAtField to the top.
Otherwise (or to get initial counts) you could do a count by starting at the root node and traversing the tree, setting the count of each node to the sum of the children and recursing. This is sort of what calculateMessageTotals does, except it only records the values for the root node, not the entire tree, even though it recurses through the entire tree.
The problem right now is that if handleMessagesAtField is called on the following tree:
A (group)
B (group)
C (group)
D (group)
calculateMessageTotals is called on node D, which traverses D to get totals
calculateMessageTotals is called on node C, which traverses C and D to get totals
calculateMessageTotals is called on node B, which traverses B,C and D to get totals
calculateMessageTotals is called on node A, which traverses A,B,C and D to get totals
so D gets calculated 4 times, C 3, and B 2
From: Brian Smith bsmith83@ad3.ucdavis.edu
Sent: Monday, July 01, 2013 3:35 PM
To: rice.usergroup.krad@kuali.org
Subject: Re:
krad.validate.js performance
I can look into the performance on this particular method, please give any suggestions you may have in improving performance here (maybe only run once somehow?).
Also, another concern is what situation is the user getting themselves into where they have 30 warnings?
Thanks,
Brian Smith
On 7/1/2013 12:10 PM, Daniel S Epstein wrote:
We’ve notice that for screens with several warning messages (~30) it is taking several 10+ seconds to run the validateForm javascript.
A lot of the time spent is in the calculateMessageTotals function, which is called recursively from handleMessagesAtField. In turn, caclculateMessageTotals calls recursiveGroupMessageCount. Because this is called recursively for each node , the calculateMessageTotals operation is called dozens of times even though it always returns the same value. These calculations should be done in O
time.
–
You received this message because you are subscribed to the Google Groups "Rice KRAD User Group" group.
Visit this group at http://groups.google.com/a/kuali.org/group/rice.usergroup.krad/.
To view this discussion on the web visit https://groups.google.com/a/kuali.org/d/msgid/rice.usergroup.krad/457ADF0F0F3CD14BB49760FE098AC209A5233A%40OITMX1004.AD.UMD.EDU.
Issue Links
- is related to
-
KSENROLL-7852
Performance: Edit ACAL Page UI hangs for 1 minute before Loading
-
Activity
After poking around a bit and reminding myself of how this was structured, I disagree with some of the suggestions made here, it is necessary for handleMessagesAtGroup and writeMessagesAtField to be called on the unhighlight call.
I still don't understand the situation you are getting into where the processing is taking 10+ seconds, we have a test page that has over 100 messages total on fields with subcollections and interacting with fields of that page does not cause a visible delay
I suspect that this delay is a combination of calling the validateForm and having that many fields that have errors simultaneously. Unhighlight/highlight is called by the plugin automatically in those situations, we could potentially skip the call on the validateForm call somehow and call it directly (similar to what we do during page setup). First I have to come up with a scenario where I can actually get the delay.
I managed to reproduce a scenario where I was getting some delay (3-4 seconds about), and have made some changes to significantly improve the performance of validateForm by somewhat mimicking logic we use elsewhere to improve the performance on initial page load. I will verify with Dan if this will solve his delay.
We have 30+ warnings because we have collections of data in our form, sometimes small changes in one part of the form can render many of the collection lines invalid.
The slowness is all in the following stack:
recursiveGroupMessageCount (krad.validate.js:848)
calculateMessageTotals (krad.validate.js:826)
handleMessagesAtGroup (krad.validate.js:485)
handleMessagesAtField (krad.validate.js:433)
jQuery.validator.setDefaults.unhighlight (krad.initialize.js:684)
$.extend.defaultShowErrors (jquery.validate.js:596)
jQuery.validator.setDefaults.showErrors (krad.initialize.js:700)
$.extend.showErrors (jquery.validate.js:365)
$.extend.form (jquery.validate.js:315)
$.extend.valid (jquery.validate.js:81)
validateForm (krad.actions.js:141)
KradRequest.send (krad.request.js:127)
unhighlight is called many times (once for each valid element), I believe that this should just unhiglight, and that the calls to handleMessagesAtGroup/Field/() and writeMessagesAtField() should be removed and their logic should only need to be called once per call to validateForm() (after default validation is complete)