javaserverfaces
  1. javaserverfaces
  2. JAVASERVERFACES-134

Optimisations to managed bean creation facility

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.2
    • Fix Version/s: 1.0
    • Component/s: None
    • Labels:
      None
    • Environment:

      Operating System: All
      Platform: All

    • Issuezilla Id:
      134

      Description

      I'm wondering if it wouldn't be more efficient to fetch a ManagedBean
      object instance from the ApplicationAssociate:

      Some Sample Code:

      // move creation logic out of associate and into ManagedBean
      ManagedBean bean = associate.getManagedBean(propertyStr);
      if (bean != null) {
      return bean.instance(facesContext);
      }

      The "instance" method encapsulates:

      • Checking the appropriate scope and returning that instance
      • If not already in scope, synchronize on that scope only and
        create/store

      Currently, the ApplicationAssociate synchronizes itself on Application
      scope and Session scope-- this isn't probably the best approach. If
      anything, it should synchronize on the Session or ServletContext object
      itself as to not lock the ApplicationAssociate for all threads.

      Also, the ELResolver implementation does 4 HashMap checks because the
      ApplicationAssociate doesn't provide access to ManagedBean data. It
      would probably be more efficient if the ApplicationAssociate was able
      to provide knowledge of a ManagedBean's existence without requiring
      redundant scope checks on each invocation.

      – Jacob

      Jayashri Visvanathan wrote:

      > Hi Jacob,
      > jacob@hookom.net wrote:
      >
      >> Also, the ELResolver implementation does 4 HashMap checks because the
      >> ApplicationAssociate doesn't provide access to ManagedBean data. It
      >> would probably be more efficient if the ApplicationAssociate was able
      >> to provide knowledge of a ManagedBean's existence without requiring
      >> redundant scope checks on each invocation.
      >>
      >>
      > I think we still need the HashMap checks because according to the spec,
      managed bean may have a property that
      > points at an object stored in a scope with a (potentially) shorter life span
      that could have been set programmatically.

      That's a good point and started me thinking about a larger issue with duplicate
      variable name use....

      For example, I could specify a SpringELResolver that already has a variable of
      the same name that would be executed after ManagedBeanELResolver which only
      checked Request/Session/Application Scope.

      Again, I think the problem has the potential of being much larger. Correcting
      it would involve changing the order of ELResolvers in the chain specified in
      JSP2.8. I would suggest pushing the the ScopedAttributeELResolver much higher
      in the chain in order to prevent redundancy checking in variable contexts that
      are added to the application. Granted, the ScopedAttributeELResolver would need
      to change its implementation slightly as to not always resolve where base ==
      null, allowing other variable contexts to resolve later.

      Another reason to push the ScopedAttributeELResolver up in the chain is because
      it represents the scope that the application programmer uses to pass information
      around. Slapping it at the end leaves a lot of room for gotchas from other
      ELResolvers and forces them to accomodate as ManagedBeanELResolver is doing.

      If you think it's a possible issue too, I would like to move this over to the EG.

        Activity

        Hide
        jayashri added a comment -

        Taking ownership

        Show
        jayashri added a comment - Taking ownership
        Hide
        jayashri added a comment -

        Fix for this issue
        M src/com/sun/faces/application/ApplicationImpl.java

        • JSF RI 134. Synchronize on Application or SessionMap instead of
          locking on ApplicationAssociate.

        Index: src/com/sun/faces/application/ApplicationAssociate.java
        ===================================================================
        RCS file:
        /cvs/javaserverfaces-sources/jsf-ri/src/com/sun/faces/application/ApplicationAssociate.java,v
        retrieving revision 1.17
        diff -u -r1.17 ApplicationAssociate.java
        — src/com/sun/faces/application/ApplicationAssociate.java 29 Jul 2005
        20:10:24 -0000 1.17
        +++ src/com/sun/faces/application/ApplicationAssociate.java 9 Aug 2005 01:14:55
        -0000
        if ((scopeIsApplication =
        scope.equalsIgnoreCase(RIConstants.APPLICATION)) ||
        (scopeIsSession = scope.equalsIgnoreCase(RIConstants.SESSION))) {

        • synchronized (this) {
        • try {
        • bean = managedBean.newInstance(context);
        • if (logger.isLoggable(Level.FINE)) {
        • logger.fine("Created bean " + managedBeanName
        • + " successfully ");
          + if (scopeIsApplication) {
          + Map applicationMap = context.getExternalContext().
          + getApplicationMap();
          + synchronized (applicationMap) {
          + try
          Unknown macro: {+ bean = managedBean.newInstance(context);+ if (logger.isLoggable(Level.FINE)) { + logger.fine("Created application scoped bean " + + managedBeanName + " successfully "); + }+ }

          catch (Exception ex) {
          + Object[] params =

          {managedBeanName};
          + if (logger.isLoggable(Level.SEVERE)) { + logger.log(Level.SEVERE, + "jsf.managed_bean_creation_error", params); + }
          + throw new FacesException(ex);
          }
          - } catch (Exception ex) {
          - Object[] params = {managedBeanName}

          ;

        • if (logger.isLoggable(Level.SEVERE)) { - logger.log(Level.SEVERE, - "jsf.managed_bean_creation_error", params); + //add bean to appropriate scope + applicationMap.put(managedBeanName, bean); + }

          + } else {
          + Map sessionMap = Util.getSessionMap(context);
          + synchronized (sessionMap) {
          + try

          Unknown macro: {+ bean = managedBean.newInstance(context);+ if (logger.isLoggable(Level.FINE)) { + logger.fine("Created session scoped bean " + + managedBeanName + " successfully "); + }+ }

          catch (Exception ex)

          Unknown macro: {+ Object[] params = {managedBeanName};+ if (logger.isLoggable(Level.SEVERE)) { + logger.log(Level.SEVERE, + "jsf.managed_bean_creation_error", params); + }+ throw new FacesException(ex); }
        • throw new FacesException(ex);
        • }
        • //add bean to appropriate scope
        • if (scopeIsApplication) { - context.getExternalContext(). - getApplicationMap().put(managedBeanName, bean); + //add bean to appropriate scope + sessionMap.put(managedBeanName, bean); }
        • if (scopeIsSession) { - Util.getSessionMap(context).put(managedBeanName, bean); - }
        • }
          + }
          } else {
          scopeIsRequest = scope.equalsIgnoreCase(RIConstants.REQUEST);
          try {
        Show
        jayashri added a comment - Fix for this issue M src/com/sun/faces/application/ApplicationImpl.java JSF RI 134. Synchronize on Application or SessionMap instead of locking on ApplicationAssociate. Index: src/com/sun/faces/application/ApplicationAssociate.java =================================================================== RCS file: /cvs/javaserverfaces-sources/jsf-ri/src/com/sun/faces/application/ApplicationAssociate.java,v retrieving revision 1.17 diff -u -r1.17 ApplicationAssociate.java — src/com/sun/faces/application/ApplicationAssociate.java 29 Jul 2005 20:10:24 -0000 1.17 +++ src/com/sun/faces/application/ApplicationAssociate.java 9 Aug 2005 01:14:55 -0000 if ((scopeIsApplication = scope.equalsIgnoreCase(RIConstants.APPLICATION)) || (scopeIsSession = scope.equalsIgnoreCase(RIConstants.SESSION))) { synchronized (this) { try { bean = managedBean.newInstance(context); if (logger.isLoggable(Level.FINE)) { logger.fine("Created bean " + managedBeanName + " successfully "); + if (scopeIsApplication) { + Map applicationMap = context.getExternalContext(). + getApplicationMap(); + synchronized (applicationMap) { + try Unknown macro: {+ bean = managedBean.newInstance(context);+ if (logger.isLoggable(Level.FINE)) { + logger.fine("Created application scoped bean " + + managedBeanName + " successfully "); + }+ } catch (Exception ex) { + Object[] params = {managedBeanName}; + if (logger.isLoggable(Level.SEVERE)) { + logger.log(Level.SEVERE, + "jsf.managed_bean_creation_error", params); + } + throw new FacesException(ex); } - } catch (Exception ex) { - Object[] params = {managedBeanName} ; if (logger.isLoggable(Level.SEVERE)) { - logger.log(Level.SEVERE, - "jsf.managed_bean_creation_error", params); + //add bean to appropriate scope + applicationMap.put(managedBeanName, bean); + } + } else { + Map sessionMap = Util.getSessionMap(context); + synchronized (sessionMap) { + try Unknown macro: {+ bean = managedBean.newInstance(context);+ if (logger.isLoggable(Level.FINE)) { + logger.fine("Created session scoped bean " + + managedBeanName + " successfully "); + }+ } catch (Exception ex) Unknown macro: {+ Object[] params = {managedBeanName};+ if (logger.isLoggable(Level.SEVERE)) { + logger.log(Level.SEVERE, + "jsf.managed_bean_creation_error", params); + }+ throw new FacesException(ex); } throw new FacesException(ex); } //add bean to appropriate scope if (scopeIsApplication) { - context.getExternalContext(). - getApplicationMap().put(managedBeanName, bean); + //add bean to appropriate scope + sessionMap.put(managedBeanName, bean); } if (scopeIsSession) { - Util.getSessionMap(context).put(managedBeanName, bean); - } } + } } else { scopeIsRequest = scope.equalsIgnoreCase(RIConstants.REQUEST); try {
        Hide
        jayashri added a comment -

        Fix checked in.

        Show
        jayashri added a comment - Fix checked in.
        Hide
        Manfred Riem added a comment -

        Closing issue out

        Show
        Manfred Riem added a comment - Closing issue out

          People

          • Assignee:
            jayashri
            Reporter:
            jayashri
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: