Details

    • Type: Task Task
    • Status: Closed Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.4
    • Security Level: Public (Public: Anyone can view)
    • Labels:
      None
    • Similar issues:
      KULRICE-3195KIM - Performance analysis and related tasks
      KULRICE-10260Analyze performance in targeted KRAD views
      KULRICE-13891Gap analysis: Agenda Editor
      KULRICE-8739Perform analysis of the KNS lookup framework
      KULRICE-10116Maintenance Gap Analysis: Permission
      KULRICE-8792Perform analysis of the KNS inquiry framework
      KULRICE-8794Perform analysis of the KNS maintenance framework
      KULRICE-8795Perform analysis of the KNS transactional framework
      KULRICE-4247Perform and document modularity analysis on KNS
    • Epic Link:
    • Rice Module:
      KRAD
    • Sprint:
      2.4.0-m3 KRAD UXI Sprint 3, 2.4.0-m3 KRAD UXI Sprint 4, 2.4.0-m4 KRAD UXI Sprint 2
    • KAI Review Status:
      Not Required
    • KTI Review Status:
      Not Required
    • Code Review Status:
      Not Required
    • Include in Release Notes?:
      Yes

      Description

      Do analysis and POC on following items:

      1. Reuse nested component lists. Initial timings with this approach several weeks ago were very positive, but I kept having to back it out due to stability issues. The lifecycle is much more organized now – once the current refactoring effort is stable, reusing nested component lists in a controlled manner will be quite a bit easier to accomplish.
      2. Incremental view indexing and cleaning. During the lifecycle, there are several full tree traversals taking place: one for each phase, one to index the entire view between phases, and another to clean the view at the end. There are also multiple smaller traversals within various lifecycle phases for different purposes – for example to search for data fields in a collection. Incremental indexing could minimize these traversals, and incremental cleaning could free resources immediately after the render phase.
      3. Component prototype “recycling”. If we can set up a reliable mechanism to recycle component instances, particularly those consumed as prototypes during the lifecycle, then heap consumption and GC overhead can be significantly reduced. The technique appears to have worked well to reverse the overhead added during lifecycle phase/task refactoring, and could work on a larger scale if applied to components. Have a look at LifecycleTaskFactory and LifecyclePhaseFactory for the general idea.

        Activity

        Hide
        Mark Fyffe (Inactive) added a comment -

        Cleanup items noted by Jerry for another pass at this issue:

        1) Quick style thing: We decided for ternary operators they should be very simple and fit on one line. For example, we would rather see this as an if:

        Component parentComponent = parent == null ? null
        : parent.getElement() instanceof Component ? (Component) parent.getElement()
        : parent.getParent();
        2) With the latest changes we moved task initialization more to the lifecycle element. I would rather keep them in the phase and have one place to look. Could we do this if we made tasks that were only applied to certain elements? In general, it would be great if we could isolate all the lifecycle code (besides the phase callbacks) to the lifecycle packages.

        3) The clearIndex(); call on View causes issues with getting components by id. For example in Action we need to get a component to set the refreshedByAction flag, but if the phase has not been run on that component yet it is not in the index:

        Action:

        else if (StringUtils.isNotBlank(refreshId)) {
        Component component = view.getViewIndex().getComponentById(refreshId);
        if (component != null)

        Unknown macro: { refreshComponent = component; }

        }

        4) We need to make another pass and cleanup javadocs and method ordering. Lots of incorrect Javadocs are present, and style problems. Does Eclipse highlight problems in the Javadoc (for example when the doc has a unmatched param tag)? Here is more on doc standards:

        https://wiki.kuali.org/display/KULRICE/Documentation+Standards

        Show
        Mark Fyffe (Inactive) added a comment - Cleanup items noted by Jerry for another pass at this issue: 1) Quick style thing: We decided for ternary operators they should be very simple and fit on one line. For example, we would rather see this as an if: Component parentComponent = parent == null ? null : parent.getElement() instanceof Component ? (Component) parent.getElement() : parent.getParent(); 2) With the latest changes we moved task initialization more to the lifecycle element. I would rather keep them in the phase and have one place to look. Could we do this if we made tasks that were only applied to certain elements? In general, it would be great if we could isolate all the lifecycle code (besides the phase callbacks) to the lifecycle packages. 3) The clearIndex(); call on View causes issues with getting components by id. For example in Action we need to get a component to set the refreshedByAction flag, but if the phase has not been run on that component yet it is not in the index: Action: else if (StringUtils.isNotBlank(refreshId)) { Component component = view.getViewIndex().getComponentById(refreshId); if (component != null) Unknown macro: { refreshComponent = component; } } 4) We need to make another pass and cleanup javadocs and method ordering. Lots of incorrect Javadocs are present, and style problems. Does Eclipse highlight problems in the Javadoc (for example when the doc has a unmatched param tag)? Here is more on doc standards: https://wiki.kuali.org/display/KULRICE/Documentation+Standards
        Hide
        Mark Fyffe (Inactive) added a comment -

        Addressed item #2:

        I have moved lifecycle task initialization back to the default methods in ComponentIntitializePhase, ComponentApplyModelPhase, and ComponentFinalizePhase. To eliminate instanceof checking, and other complex branching logic during this setup, I added a check while executing tasks to ensure that the lifecycle element for the current phase is an instance of an "elementClass" defined for the task.

        I've switched development over to IntelliJ to complete a final pass at JavaDoc cleanup. TODOs from 12/23 are still outstanding.

        Show
        Mark Fyffe (Inactive) added a comment - Addressed item #2: I have moved lifecycle task initialization back to the default methods in ComponentIntitializePhase, ComponentApplyModelPhase, and ComponentFinalizePhase. To eliminate instanceof checking, and other complex branching logic during this setup, I added a check while executing tasks to ensure that the lifecycle element for the current phase is an instance of an "elementClass" defined for the task. I've switched development over to IntelliJ to complete a final pass at JavaDoc cleanup. TODOs from 12/23 are still outstanding.
        Hide
        Mark Fyffe (Inactive) added a comment -

        Work is in progress on the 2.4 performance branch for converting the pre-process phase to a fully formed lifecycle phase.

        Implementation is fully complete, but I am working through a few null pointers - related to the model being null during the pre-process phase.

        I expect to have this issue resolved early this week.

        Show
        Mark Fyffe (Inactive) added a comment - Work is in progress on the 2.4 performance branch for converting the pre-process phase to a fully formed lifecycle phase. Implementation is fully complete, but I am working through a few null pointers - related to the model being null during the pre-process phase. I expect to have this issue resolved early this week.
        Hide
        Mark Fyffe (Inactive) added a comment -

        Committed updates to performance branch:

        1. Added "viewPath" bean property to both LifecycleElement and ViewLifecyclePhase for the full path relative to the view, and clarified the use of the "path" property as relative to the element's parent component.
        2. Converted pre-process to a fully formed lifecycle phase.
        3. Added recycling to lifecycle element maps.

        Last step is to investigate impact related to clearIndex()

        Show
        Mark Fyffe (Inactive) added a comment - Committed updates to performance branch: Added "viewPath" bean property to both LifecycleElement and ViewLifecyclePhase for the full path relative to the view, and clarified the use of the "path" property as relative to the element's parent component. Converted pre-process to a fully formed lifecycle phase. Added recycling to lifecycle element maps. Last step is to investigate impact related to clearIndex()
        Hide
        Mark Fyffe (Inactive) added a comment -

        Removed clearIndex() calls and performed regression testing to check for fallout. With the planned removal of ViewIndex, this should have minimal long-term impact.

        As a final item for this issue, I modified ProcessLoggingUnitTest to facilitate multiple repetitions of the test. It is up to the test itself to alter the test expectations for each run. I set each ViewLifecycleTest method to run 2 times: once with asynchronous lifecycle, and once synchronous.

        Show
        Mark Fyffe (Inactive) added a comment - Removed clearIndex() calls and performed regression testing to check for fallout. With the planned removal of ViewIndex, this should have minimal long-term impact. As a final item for this issue, I modified ProcessLoggingUnitTest to facilitate multiple repetitions of the test. It is up to the test itself to alter the test expectations for each run. I set each ViewLifecycleTest method to run 2 times: once with asynchronous lifecycle, and once synchronous.

          People

          • Assignee:
            Mark Fyffe (Inactive)
            Reporter:
            Jerry Neal (Inactive)
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Time Tracking

              Estimated:
              Original Estimate - 2 days Original Estimate - 2 days
              2d
              Remaining:
              Remaining Estimate - 0 minutes
              0m
              Logged:
              Time Spent - 1 week, 7 hours
              1w 7h

                Agile

                  Structure Helper Panel