Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.0-Alpha2
    • Fix Version/s: 2.0-Beta1, 2.0.0
    • Component/s: Framework
    • Labels:
      None
    • Environment:
      ICEfaces, session replication

      Description

      Many of the Objects maintained by ICEfaces in the Session are likely unnecessary and can be instantiated as needed in the request cycle. This should be optimized to reduce unnecessary object replication in cluster environments.

        Issue Links

          Activity

          Hide
          Ted Goddard added a comment -

          Initial work will be to survey existing Objects in Session scope and comment on alternatives.

          Show
          Ted Goddard added a comment - Initial work will be to survey existing Objects in Session scope and comment on alternatives.
          Hide
          Ken Fyten added a comment -

          Need to complete the survey so it can be reviewed/implemented by Mircea, Deryk.

          Show
          Ken Fyten added a comment - Need to complete the survey so it can be reviewed/implemented by Mircea, Deryk.
          Hide
          Deryk Sinotte added a comment -

          Perhaps having a single root object (ICEfacesSessionContext) that handles all session-based interaction for ICEfaces. This object could be exposed to provide an API as well as a central place for all logging, profiling, etc.

          Show
          Deryk Sinotte added a comment - Perhaps having a single root object (ICEfacesSessionContext) that handles all session-based interaction for ICEfaces. This object could be exposed to provide an API as well as a central place for all logging, profiling, etc.
          Hide
          Ted Goddard added a comment -

          Each of the following should be examined for necessity in being in the Session and for impact on Serialization in clustering.

          Session Attribute: org.icefaces.application.LazyPushManager
          toString(): org.icefaces.application.LazyPushManager@33d232d1

          Session Attribute: org.icefaces.push.SessionViewManager
          toString(): org.icefaces.push.SessionViewManager@380fe8c4

          Session Attribute: org.icefaces.push.SessionRenderer
          toString(): org.icefaces.push.SessionViewManager@380fe8c4

          Session Attribute: org.icefaces.application.WindowScopeManager
          toString(): org.icefaces.application.WindowScopeManager@2cdb03a1

          Session Attribute: org.icefaces.push.DynamicResourceDispatcher
          toString(): org.icefaces.push.DynamicResourceDispatcher@63dd8136

          Show
          Ted Goddard added a comment - Each of the following should be examined for necessity in being in the Session and for impact on Serialization in clustering. Session Attribute: org.icefaces.application.LazyPushManager toString(): org.icefaces.application.LazyPushManager@33d232d1 Session Attribute: org.icefaces.push.SessionViewManager toString(): org.icefaces.push.SessionViewManager@380fe8c4 Session Attribute: org.icefaces.push.SessionRenderer toString(): org.icefaces.push.SessionViewManager@380fe8c4 Session Attribute: org.icefaces.application.WindowScopeManager toString(): org.icefaces.application.WindowScopeManager@2cdb03a1 Session Attribute: org.icefaces.push.DynamicResourceDispatcher toString(): org.icefaces.push.DynamicResourceDispatcher@63dd8136
          Hide
          Ted Goddard added a comment -

          Session Attribute: org.icefaces.application.WindowScopeManager

          The Hashtable of Window Scopes is appropriate for maintaining within the session since a Window Scope is effectively a sub-scope of the Session Scope (just restricted to a particular browser window). The other fields used by the WindowScopeManager are not readily propagated across the cluster: lists of disposed scopes, Observables for scope create/destroy, and the dependency on SessionBoundServer. The Observables should be factored into the WindowScopeManager directly.

          Show
          Ted Goddard added a comment - Session Attribute: org.icefaces.application.WindowScopeManager The Hashtable of Window Scopes is appropriate for maintaining within the session since a Window Scope is effectively a sub-scope of the Session Scope (just restricted to a particular browser window). The other fields used by the WindowScopeManager are not readily propagated across the cluster: lists of disposed scopes, Observables for scope create/destroy, and the dependency on SessionBoundServer. The Observables should be factored into the WindowScopeManager directly.
          Hide
          Ted Goddard added a comment -

          Session Attribute: org.icefaces.push.DynamicResourceDispatcher

          Dynamic resource lifespan should be under control of the application. There are valid cases for resources being maintained in nearly any scope (except Request scope would not work as you would want: a resource kept for just that one page load would be discarded from Request scope before it could be retrieved).

          In other words, the DynamicResourceDispatcher itself does not need to be maintained in Session scope, but Resources should be created and placed in an appropriate scope and a ResourceHandler would use the dynamic resource URL together with scope resolution to return the desired resource bytes.

          Show
          Ted Goddard added a comment - Session Attribute: org.icefaces.push.DynamicResourceDispatcher Dynamic resource lifespan should be under control of the application. There are valid cases for resources being maintained in nearly any scope (except Request scope would not work as you would want: a resource kept for just that one page load would be discarded from Request scope before it could be retrieved). In other words, the DynamicResourceDispatcher itself does not need to be maintained in Session scope, but Resources should be created and placed in an appropriate scope and a ResourceHandler would use the dynamic resource URL together with scope resolution to return the desired resource bytes.
          Hide
          Ted Goddard added a comment -

          Session Attribute: org.icefaces.push.SessionRenderer

          No matter what, this class should be renamed since it introduces confusion with the SessionRenderer API in ICEfaces 1.8 (however, it looks like we should remove this class and replace it with ICEpush functionality).

          For efficiency, "session" rendering should be introduced to ICEpush via a reserved SUBID.

          Show
          Ted Goddard added a comment - Session Attribute: org.icefaces.push.SessionRenderer No matter what, this class should be renamed since it introduces confusion with the SessionRenderer API in ICEfaces 1.8 (however, it looks like we should remove this class and replace it with ICEpush functionality). For efficiency, "session" rendering should be introduced to ICEpush via a reserved SUBID.
          Hide
          Ted Goddard added a comment -

          Session Attribute: org.icefaces.push.SessionViewManager

          Should be replaced with ICEpush functionality, since this is just a different access point to org.icefaces.push.SessionRenderer.

          Show
          Ted Goddard added a comment - Session Attribute: org.icefaces.push.SessionViewManager Should be replaced with ICEpush functionality, since this is just a different access point to org.icefaces.push.SessionRenderer.
          Hide
          Ted Goddard added a comment -

          Session Attribute: org.icefaces.application.LazyPushManager

          Maintains list of view keys (not viewID) for views making use of push.

          When failover occurs, push group membership should automatically fail over.

          If possible, ICEfaces should avoid duplicating JSF data structures (the table of view keys is a partial duplication of JSF state management).

          PushRenderer.addCurrentSession()

          • set a Session attribute containing the PUSHID for the session. If this session attribute is detected during page rendering, add the ICEpush JavaScript to the page.

          PushRenderer.addCurrentView()

          • set a viewMap attribute containing the PUSHID for the view. If this view attribute is detected during page rendering, add the ICEpush JavaScript to the page.
          Show
          Ted Goddard added a comment - Session Attribute: org.icefaces.application.LazyPushManager Maintains list of view keys (not viewID) for views making use of push. When failover occurs, push group membership should automatically fail over. If possible, ICEfaces should avoid duplicating JSF data structures (the table of view keys is a partial duplication of JSF state management). PushRenderer.addCurrentSession() set a Session attribute containing the PUSHID for the session. If this session attribute is detected during page rendering, add the ICEpush JavaScript to the page. PushRenderer.addCurrentView() set a viewMap attribute containing the PUSHID for the view. If this view attribute is detected during page rendering, add the ICEpush JavaScript to the page.
          Hide
          Ted Goddard added a comment -

          One advantage of the use of JDK types in session attributes is that they are easily displayed by debugging tools, such as the tomcat manager. (So, rather than create an ICESessionContext object, we may be well served by cutting back on the number of session attributes and simplifying their representation.)

          Show
          Ted Goddard added a comment - One advantage of the use of JDK types in session attributes is that they are easily displayed by debugging tools, such as the tomcat manager. (So, rather than create an ICESessionContext object, we may be well served by cutting back on the number of session attributes and simplifying their representation.)
          Hide
          Ted Goddard added a comment -

          Please review and comment.

          Show
          Ted Goddard added a comment - Please review and comment.
          Hide
          Ted Goddard added a comment -

          Note that for ICEpush, session-based rendering is not inherent – a notification to a BROWSERID could cover notifications to other applications as well, which may not be desired.

          Show
          Ted Goddard added a comment - Note that for ICEpush, session-based rendering is not inherent – a notification to a BROWSERID could cover notifications to other applications as well, which may not be desired.
          Hide
          Mircea Toma added a comment - - edited

          Ted said:
          "Session Attribute: org.icefaces.push.SessionViewManager
          Should be replaced with ICEpush functionality, since this is just a different access point to org.icefaces.push.SessionRenderer."

          SessionViewManager is the link between ICEfaces and ICEpush. It keeps track of the views that belong to the current session, when PushRenderer.addCurrentSession(group) is called it adds the current views to the named group by using ICEpush API (PushContext.addGroupMember). Since PushRenderer.addCurrentSession or PushRenderer.removeCurrentSession can be called any time during the lifespan of the application it is necessary to keep track of the views created or destroyed in the interim, PushRenderer cannot just delegate directly the management of these pushIDs to PushContext.

          Show
          Mircea Toma added a comment - - edited Ted said: "Session Attribute: org.icefaces.push.SessionViewManager Should be replaced with ICEpush functionality, since this is just a different access point to org.icefaces.push.SessionRenderer." SessionViewManager is the link between ICEfaces and ICEpush. It keeps track of the views that belong to the current session, when PushRenderer.addCurrentSession(group) is called it adds the current views to the named group by using ICEpush API (PushContext.addGroupMember). Since PushRenderer.addCurrentSession or PushRenderer.removeCurrentSession can be called any time during the lifespan of the application it is necessary to keep track of the views created or destroyed in the interim, PushRenderer cannot just delegate directly the management of these pushIDs to PushContext.
          Hide
          Mircea Toma added a comment -

          Ted said:
          "Session Attribute: org.icefaces.push.SessionRenderer
          No matter what, this class should be renamed since it introduces confusion with the SessionRenderer API in ICEfaces 1.8 (however, it looks like we should remove this class and replace it with ICEpush functionality)."

          This class seems to be legacy code that was used initially to integrate ICEpush into ICEfaces. PushRenderer has replaced and extended its functionality. org.icefaces.push.SessionRenderer session attribute key points to the instance of org.icefaces.push.SessionViewManager, since org.icefaces.push.SessionViewManager implements the interface.

          Show
          Mircea Toma added a comment - Ted said: "Session Attribute: org.icefaces.push.SessionRenderer No matter what, this class should be renamed since it introduces confusion with the SessionRenderer API in ICEfaces 1.8 (however, it looks like we should remove this class and replace it with ICEpush functionality)." This class seems to be legacy code that was used initially to integrate ICEpush into ICEfaces. PushRenderer has replaced and extended its functionality. org.icefaces.push.SessionRenderer session attribute key points to the instance of org.icefaces.push.SessionViewManager, since org.icefaces.push.SessionViewManager implements the interface.
          Hide
          Mircea Toma added a comment -

          Ted said:

          "Session Attribute: org.icefaces.application.LazyPushManager

          Maintains list of view keys (not viewID) for views making use of push.

          When failover occurs, push group membership should automatically fail over.

          If possible, ICEfaces should avoid duplicating JSF data structures (the table of view keys is a partial duplication of JSF state management)."

          It is a duplication of IDs, that's all. The reason is reasonably well explained in http://jira.icefaces.org/browse/ICE-5594?focusedCommentId=28866&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#action_28866 comment.

          "PushRenderer.addCurrentSession()

          • set a Session attribute containing the PUSHID for the session. If this session attribute is detected during page rendering, add the ICEpush JavaScript to the page."

          This strategy doesn't hold if PushRenderer.addCurrentSession is invoked a number of times and different from how many times PushRenderer.removeCurrentSession is called. That's the reason for the "noOfRegistrations" counter in LazyPushManager. Yes, the counter can be kept directly into the session, but implementation wise you end up with lots of session attributes that the different "managers" will use – global variable syndrome once again....which I hate.

          "PushRenderer.addCurrentView()

          • set a viewMap attribute containing the PUSHID for the view. If this view attribute is detected during page rendering, add the ICEpush JavaScript to the page."

          The viewMap is not the right container since the viewIDs are actually 1-to-1 mapped to the view state keys. The view state keys represent the state of the different views currently active for a session.

          In the end LazyPushManager could be merged with SessionViewManager, but really its functionality is orthogonal to the one of SessionViewManager.

          Show
          Mircea Toma added a comment - Ted said: "Session Attribute: org.icefaces.application.LazyPushManager Maintains list of view keys (not viewID) for views making use of push. When failover occurs, push group membership should automatically fail over. If possible, ICEfaces should avoid duplicating JSF data structures (the table of view keys is a partial duplication of JSF state management)." It is a duplication of IDs, that's all. The reason is reasonably well explained in http://jira.icefaces.org/browse/ICE-5594?focusedCommentId=28866&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#action_28866 comment. "PushRenderer.addCurrentSession() set a Session attribute containing the PUSHID for the session. If this session attribute is detected during page rendering, add the ICEpush JavaScript to the page." This strategy doesn't hold if PushRenderer.addCurrentSession is invoked a number of times and different from how many times PushRenderer.removeCurrentSession is called. That's the reason for the "noOfRegistrations" counter in LazyPushManager. Yes, the counter can be kept directly into the session, but implementation wise you end up with lots of session attributes that the different "managers" will use – global variable syndrome once again....which I hate. "PushRenderer.addCurrentView() set a viewMap attribute containing the PUSHID for the view. If this view attribute is detected during page rendering, add the ICEpush JavaScript to the page." The viewMap is not the right container since the viewIDs are actually 1-to-1 mapped to the view state keys. The view state keys represent the state of the different views currently active for a session. In the end LazyPushManager could be merged with SessionViewManager, but really its functionality is orthogonal to the one of SessionViewManager.
          Hide
          Mircea Toma added a comment - - edited

          All these objects need access to objects that can be constructed only locally, such as PushContext or ServletContext. The managers should be changed so that they won't keep references to instances of ServletContext and such, but they would receive them when their methods are invoked.
          For example SessionViewManager.addView(String id) would be change to SessionViewManager.addView(String id, PushContext context) since the method uses internally PushContext class.
          Also the fields of the managers can be saved into a serializable structure (without behavior) and that will be saved into the session (thus session scoped).
          This approach will allow the creation of the managers at startup time, the context they would work in would be changed depending on what context instance it's passed in.

          Show
          Mircea Toma added a comment - - edited All these objects need access to objects that can be constructed only locally, such as PushContext or ServletContext. The managers should be changed so that they won't keep references to instances of ServletContext and such, but they would receive them when their methods are invoked. For example SessionViewManager.addView(String id) would be change to SessionViewManager.addView(String id, PushContext context) since the method uses internally PushContext class. Also the fields of the managers can be saved into a serializable structure (without behavior) and that will be saved into the session (thus session scoped). This approach will allow the creation of the managers at startup time, the context they would work in would be changed depending on what context instance it's passed in.
          Hide
          Mircea Toma added a comment -

          Removed obsolete org.icefaces.push.SessionRenderer API and implementation (not to be confused with the SessionRenderer API in "compat"). This API was used initailly for integrating ICEpush with ICEfaces. Later on was replaced by PushRenderer.

          Show
          Mircea Toma added a comment - Removed obsolete org.icefaces.push.SessionRenderer API and implementation (not to be confused with the SessionRenderer API in "compat"). This API was used initailly for integrating ICEpush with ICEfaces. Later on was replaced by PushRenderer.
          Hide
          Mircea Toma added a comment - - edited

          Refactored WindowScopeManager, SessionViewManager and LazyPushManager to save their serializable state in the session. Changed its methods to be stateless (and static), state is accessed and modified by these methods by looking it up from the session.

          Show
          Mircea Toma added a comment - - edited Refactored WindowScopeManager, SessionViewManager and LazyPushManager to save their serializable state in the session. Changed its methods to be stateless (and static), state is accessed and modified by these methods by looking it up from the session.
          Hide
          Mircea Toma added a comment -

          Created a separate issue for org.icefaces.push.DynamicResourceDispatcher refactoring since its resolution entails changes in the resource registration behavior: ICE-5848

          Show
          Mircea Toma added a comment - Created a separate issue for org.icefaces.push.DynamicResourceDispatcher refactoring since its resolution entails changes in the resource registration behavior: ICE-5848

            People

            • Assignee:
              Mircea Toma
              Reporter:
              Ted Goddard
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: