Uploaded image for project: 'Kuali Rice Development'
  1. Kuali Rice Development
  2. KULRICE-9866

Performance issue in calculate message totals on client

    Details

    • Type: Task
    • Status: Closed
    • Priority: Blocker
    • Resolution: Fixed
    • Affects Version/s: 2.3
    • Fix Version/s: 2.3.0-rc1, 2.3
    • Component/s: User Interface
    • Security Level: Public (Public: Anyone can view)
    • Labels:
      None
    • 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:

      {Rice KRAD User Group}

      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.

        Attachments

          Issue Links

            Activity

            Hide
            depstein Daniel Epstein (Inactive) added a comment -

            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)

            Show
            depstein Daniel Epstein (Inactive) added a comment - 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)
            Hide
            bsmith Brian Smith (Inactive) added a comment -

            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.

            Show
            bsmith Brian Smith (Inactive) added a comment - 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.
            Hide
            bsmith Brian Smith (Inactive) added a comment - - edited

            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.

            Show
            bsmith Brian Smith (Inactive) added a comment - - edited 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.

              People

              • Assignee:
                bsmith Brian Smith (Inactive)
                Reporter:
                bsmith Brian Smith (Inactive)
              • Votes:
                0 Vote for this issue
                Watchers:
                2 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: