Details
-
Type:
Task
-
Status: Closed
-
Priority:
Major
-
Resolution: Fixed
-
Affects Version/s: None
-
Fix Version/s: 2.4
-
Component/s: Development, User Experience (UX)
-
Security Level: Public (Public: Anyone can view)
-
Labels:None
-
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.
Attachments
Activity
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
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.
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.
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()
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.
Implemented CopyUtils and RecycleUtils to facilitate more efficient copy with copyProperties() and reuse of a recycling object factory pattern.
The clone-based copy is faster (roughly 10-20% in unit test and krad-sampleapp runs) than the original Copyable implementation, and does not require implementations to write a copyProperties() method. To enable this feature, set "rice.krad.copyable.useClone" to true. If this new copy implementation passes review and is proven stable, then it may be become the default to simplify component authoring while improving performance overall.
RecycleUtils provides a thread-local queue of objects that may be reused by later operations in the same thread. Objects that have been recycled are held by weak references to prevent heap consumption. When several objects are used then discarded in a short-term fashion, then recycling eliminates constructor and GC overhead since the same instances may be reused.
These two updates are being committed along with a larger updated on
KULRICE-10550.