Kuali Rice Development
  1. Kuali Rice Development
  2. KULRICE-9866

Performance issue in calculate message totals on client

    Details

    • Type: Task Task
    • Status: Closed Closed
    • Priority: Blocker 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
    • Similar issues:
      KULRICE-8324Performance improvements for totalling and group totalling
      KULRICE-10093Validation 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-13524Rice 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:

      {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.

        Issue Links

          Activity

          Hide
          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
          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
          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
          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
          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
          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:
              Brian Smith (Inactive)
              Reporter:
              Brian Smith (Inactive)
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Structure Helper Panel