myfaces-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From awi...@apache.org
Subject svn commit: r567161 - in /myfaces/trinidad/trunk/trinidad/trinidad-api/src/main/java/org/apache/myfaces/trinidad/component: StampState.java UIXCollection.java
Date Fri, 17 Aug 2007 22:30:59 GMT
Author: awiner
Date: Fri Aug 17 15:30:54 2007
New Revision: 567161

URL: http://svn.apache.org/viewvc?view=rev&rev=567161
Log:
TRINIDAD-630: Table stamp state could be greatly optimized for size
- Stop storing state that is nothing but a hierarchy of Object[] arrays where all the entries
are null
- Share the state objects for showDetails - since they're either true or false, no need for
new instances each time
- Optimize state arrays further to handle common cases of leaf components or components with
children but no facets

Modified:
    myfaces/trinidad/trunk/trinidad/trinidad-api/src/main/java/org/apache/myfaces/trinidad/component/StampState.java
    myfaces/trinidad/trunk/trinidad/trinidad-api/src/main/java/org/apache/myfaces/trinidad/component/UIXCollection.java

Modified: myfaces/trinidad/trunk/trinidad/trinidad-api/src/main/java/org/apache/myfaces/trinidad/component/StampState.java
URL: http://svn.apache.org/viewvc/myfaces/trinidad/trunk/trinidad/trinidad-api/src/main/java/org/apache/myfaces/trinidad/component/StampState.java?view=diff&rev=567161&r1=567160&r2=567161
==============================================================================
--- myfaces/trinidad/trunk/trinidad/trinidad-api/src/main/java/org/apache/myfaces/trinidad/component/StampState.java
(original)
+++ myfaces/trinidad/trunk/trinidad/trinidad-api/src/main/java/org/apache/myfaces/trinidad/component/StampState.java
Fri Aug 17 15:30:54 2007
@@ -94,8 +94,6 @@
   public static Object saveStampState(FacesContext context, UIComponent stamp)
   {
     RowState state = _createState(stamp);
-    if (state != null)
-      state.saveRowState(stamp);
     return state;
   }
 
@@ -119,45 +117,80 @@
   }
 
   /**
-   * save the stamp state of the children of the given column in the given table.
+   * save the stamp state of just the children of the given component
+   * in the given table.
    */
   @SuppressWarnings("unchecked")
   public static Object saveChildStampState(
     FacesContext context,
-    UIComponent column,
+    UIComponent   stamp,
     UIXCollection table)
   {
-    List<UIComponent> kids = column.getChildren();
-    int sz = kids.size();
-    Object[] state = new Object[sz];
-    boolean wasAllTransient = true;
-    for(int i=0; i<sz; i++)
-    {
-      Object childState = table.saveStampState(context, kids.get(i));
-      state[i] = childState;
-      if (childState != UIXCollection.Transient.TRUE)
-        wasAllTransient = false;
-    }
-    
-    // If all we found were transient components, just use
-    // an empty array
-    if (wasAllTransient)
-      return _EMPTY_ARRAY;
+    int childCount = stamp.getChildCount();
+    // If we have any children, iterate through the array,
+    // saving state
+    if (childCount == 0)
+      return null;
+
+    Object[] childStateArray = null;
+    List<UIComponent> children = stamp.getChildren();
+    boolean childStateIsEmpty = true;
+    for(int i=0; i < childCount; i++)
+    {
+      UIComponent child = children.get(i);
+      Object childState = table.saveStampState(context, child);
+
+      // Until we have one non-null entry, don't allocate the array.
+      // Unlike facets, we *do* care about stashing Transient.TRUE,
+      // because we have to keep track of them relative to any
+      // later components, BUT if it's all null and transient, we 
+      // can discard the array.  This does mean that putting 
+      // transient components into a stamp is a bit inefficient
+      
+      // So: allocate the array if we encounter our first
+      // non-null childState (even if it's transient)
+      if (childStateArray == null)
+      {
+        if (childState == null)
+          continue;
+        
+        childStateArray = new Object[childCount];
+      }
+      
+      // But remember the moment we've encountered a non-null
+      // *and* non-transient component, because that means we'll
+      // really need to keep this array
+      if ((childState != UIXCollection.Transient.TRUE) && (childState != null))
+        childStateIsEmpty = false;
+      
+      // Store a value into the array
+      assert(childStateArray != null);
+      childStateArray[i] = childState;
+    }
+
+    // Even if we bothered to allocate an array, if all we 
+    // had were transient + null, don't bother with the array at all
+    if (childStateIsEmpty)
+      return null;
 
-    return state;
+    return childStateArray;
   }
 
   /**
-   * restore the stamp state of the children of the given column in the given table.
+   * Restore the stamp state of just the children of the given component
+   * in the given table.
    */
   @SuppressWarnings("unchecked")
   public static void restoreChildStampState(
     FacesContext context,
-    UIComponent column,
+    UIComponent stamp,
     UIXCollection table,
     Object stampState)
   {
-    List<UIComponent> kids = column.getChildren();
+    if (stampState == null)
+      return;
+
+    List<UIComponent> kids = stamp.getChildren();
     Object[] state = (Object[]) stampState;
 
     int childIndex = 0;
@@ -241,13 +274,27 @@
 
   private static RowState _createState(UIComponent child)
   {
+    RowState state;
     if (child instanceof EditableValueHolder)
-      return new EVHState();
-    if (child instanceof UIXShowDetail)
-      return new SDState();
-    if (child instanceof UIXCollection)
-      return new TableState();
-    return null;
+    {
+      state = new EVHState();
+      state.saveRowState(child);
+    }
+    else if (child instanceof UIXCollection)
+    {
+      state = new TableState();
+      state.saveRowState(child);
+    }
+    else if (child instanceof UIXShowDetail)
+    {
+      state = SDState.getState((UIXShowDetail) child);
+    }
+    else
+    {
+      state = null;
+    }
+
+    return state;
   }
 
   private static boolean _eq(Object k1, Object k2)
@@ -274,10 +321,26 @@
 
   static private final class SDState extends RowState
   {
+    /**
+     * Return cached, shared instances of SDState.
+     */
+    static public RowState getState(UIXShowDetail child)
+    {
+      if (child.isDisclosed())
+        return _TRUE;
+      else
+        return _FALSE;
+    }
+
     public SDState()
     {
     }
 
+    private SDState(boolean disclosed)
+    {
+      _disclosed = disclosed;
+    }
+
     @Override
     public void saveRowState(UIComponent child)
     {
@@ -301,6 +364,11 @@
     {
       return "SDState[disclosed=" + _disclosed + "]";
     }
+
+    // Reusable instances of SDState. TODO: use readResolve/writeReplace
+    // so that we only send across and restore instances of these
+    static private final SDState _TRUE = new SDState(true);
+    static private final SDState _FALSE = new SDState(false);
 
     /**
      * 

Modified: myfaces/trinidad/trunk/trinidad/trinidad-api/src/main/java/org/apache/myfaces/trinidad/component/UIXCollection.java
URL: http://svn.apache.org/viewvc/myfaces/trinidad/trunk/trinidad/trinidad-api/src/main/java/org/apache/myfaces/trinidad/component/UIXCollection.java?view=diff&rev=567161&r1=567160&r2=567161
==============================================================================
--- myfaces/trinidad/trunk/trinidad/trinidad-api/src/main/java/org/apache/myfaces/trinidad/component/UIXCollection.java
(original)
+++ myfaces/trinidad/trunk/trinidad/trinidad-api/src/main/java/org/apache/myfaces/trinidad/component/UIXCollection.java
Fri Aug 17 15:30:54 2007
@@ -780,58 +780,86 @@
     if (stamp.isTransient())
       return Transient.TRUE;
 
-    Object[] state = new Object[3];
-    state[0] = StampState.saveStampState(context, stamp);
+    // The structure we will use is:
+    //   0: state of the stamp
+    //   1: state of the children (an array)
+    //   2: state of the facets (an array of name-key pairs)
+    // If there is no facet state, we have a two-element array
+    // If there is no facet state or child state, we have a one-elment array
+    // If there is no state at all, we return null
+
+    Object stampState = StampState.saveStampState(context, stamp);
+
+    // StampState can never EVER be an Object array, as if we do,
+    // we have no possible way of identifying the difference between
+    // just having stamp state, and having stamp state + child/facet state
+    assert(!(stampState instanceof Object[]));
 
     int facetCount = _getFacetCount(stamp);
-    Object[] facetState;
-    if (facetCount == 0)
-      facetState = _EMPTY_ARRAY;
-    else
+    int childCount = stamp.getChildCount();
+
+    Object[] state = null;
+
+    if (facetCount > 0)
     {
-      facetState = new Object[facetCount * 2];
+      boolean facetStateIsEmpty = true;
+      Object[] facetState = null;
+
       Map<String, UIComponent> facetMap = stamp.getFacets();
+
       int i = 0;
       for(Map.Entry<String, UIComponent> entry : facetMap.entrySet())
       {
+        Object singleFacetState = saveStampState(context, entry.getValue());
+        // Don't bother allocating anything until we have some non-null
+        // and non-transient facet state
+        if (facetStateIsEmpty)
+        {
+          if ((singleFacetState == null) ||
+              (singleFacetState == Transient.TRUE))
+            continue;
+          
+          facetStateIsEmpty = false;
+          facetState = new Object[facetCount * 2];
+        }
+
         int base = i * 2;
+        assert(facetState != null);
         facetState[base] = entry.getKey();
-        facetState[base + 1] = saveStampState(context, entry.getValue());
+        facetState[base + 1] = singleFacetState;
         i++;
       }
-    }
-
-    state[1] = facetState;
 
-    int childCount = stamp.getChildCount();
-    Object[] childStateArray;
-    if (childCount == 0)
-    {
-      childStateArray = _EMPTY_ARRAY;
-    }
-    else
-    {
-      List<UIComponent> children = stamp.getChildren();
-      childStateArray = new Object[childCount];
-      boolean wasAllTransient = true;
-      for(int i=0; i < childCount; i++)
+      // OK, we had something:  allocate the state array to three
+      // entries, and insert the facet state at position 2
+      if (!facetStateIsEmpty)
       {
-        UIComponent child = children.get(i);
-        Object childState = saveStampState(context, child);
-        if (childState != Transient.TRUE)
-          wasAllTransient = false;
-        
-        childStateArray[i] = childState;
+        state = new Object[3];
+        state[2] = facetState;
       }
-      
-      // If all we found were transient components, just use
-      // an empty array
-      if (wasAllTransient)
-        childStateArray = _EMPTY_ARRAY;
     }
 
-    state[2] = childStateArray;
+    // If we have any children, iterate through the array,
+    // saving state
+    Object childState = StampState.saveChildStampState(context,
+                                                       stamp,
+                                                       this);
+    if (childState != null)
+    {
+      // If the state hasn't been allocated yet, we only
+      // need a two-element array
+      if (state == null)
+        state = new Object[2];
+      state[1] = childState;
+    }
+
+    // If we don't have an array, just return the stamp
+    // state
+    if (state == null)
+      return stampState;
 
+    // Otherwise, store the stamp state at index 0, and return
+    state[0] = stampState;
     return state;
   }
 
@@ -845,58 +873,52 @@
   protected void restoreStampState(FacesContext context, UIComponent stamp,
                                    Object stampState)
   {
-    if (stampState == Transient.TRUE)
+    // Just a transient component - return
+    if ((stampState == Transient.TRUE) || (stampState == null))
       return;
     
+    // If this isn't an Object array, then it's a component with state
+    // of its own, but no child/facet state - so restore and be done
+    if (!(stampState instanceof Object[]))
+    {
+      StampState.restoreStampState(context, stamp, stampState);
+      // NOTE early return
+      return;
+    }
+
     Object[] state = (Object[]) stampState;
+    int stateSize = state.length;
+    // We always have at least one element if we get to here
+    assert(stateSize >= 1);
+
     StampState.restoreStampState(context, stamp, state[0]);
 
-    Object[] facetStateArray = (Object[]) state[1];
-    for(int i=0; i<facetStateArray.length; i+=2)
+
+    // If there's any facet state, restore it
+    if (stateSize >= 3)
     {
-      String facetName = (String) facetStateArray[i];
-      Object facetState = facetStateArray[i + 1];
-      if (facetState != Transient.TRUE)
-        restoreStampState(context, stamp.getFacet(facetName), facetState);
-    }
-
-    List<UIComponent> children = stamp.getChildren();
-    Object[] childStateArray = (Object[]) state[2];
-    int childIndex = 0;
-    for(int i=0; i<childStateArray.length; i++)
-    {
-      Object childState = childStateArray[i];
-      // Skip over any saved state that corresponds to transient
-      // components
-      if (childState != Transient.TRUE)
-      {
-        while (childIndex < children.size())
-        {
-          UIComponent child = children.get(childIndex);
-          childIndex++;
-          // Skip over any transient components before restoring state
-          if (!child.isTransient())
-          {
-            restoreStampState(context, child, childState);
-            break;
-          }
-        }
-      }
-      // The component may or may not still be there;  if it
-      // is, then we'd better skip over it
-      else
+      Object[] facetStateArray = (Object[]) state[2];
+      // This had better be non-null, otherwise we never 
+      // should have allocated a three-element array!
+      assert(facetStateArray != null);
+
+      for(int i=0; i<facetStateArray.length; i+=2)
       {
-        if (childIndex < children.size())
-        {
-          UIComponent child = children.get(childIndex);
-          // If the child isn't transient, then it must be
-          // something that we want to look at on the next
-          // iteration.
-          if (child.isTransient())
-            childIndex++;
-        }
+        String facetName = (String) facetStateArray[i];
+        Object facetState = facetStateArray[i + 1];
+        if (facetState != Transient.TRUE)
+          restoreStampState(context, stamp.getFacet(facetName), facetState);
       }
     }
+    
+    // If there's any child state, restore it
+    if (stateSize >= 2)
+    {
+      StampState.restoreChildStampState(context,
+                                        stamp,
+                                        this,
+                                        state[1]);
+    }
   }
 
   /**
@@ -1188,11 +1210,12 @@
       {
         Object iniStateObj = _getCurrencyKeyForInitialStampState();
         state = stampState.get(iniStateObj, stampId);
+        /*
         if (state==null)
         {
           _LOG.severe("NO_INITIAL_STAMP_STATE", new Object[]{currencyObj,iniStateObj,stampId});
           continue;
-        }
+        }*/
       }
       restoreStampState(context, stamp, state);
     }
@@ -1393,8 +1416,9 @@
 
   // An enum to throw into state-saving so that we get a nice
   // instance-equality to test against for noticing transient components
-  // TODO: is this really better than just, say, using null for 
-  // transient components?  It's certainly better at not papering
-  // over error cases
+  // (and better serialization results)
+  // We need this instead of just using null - because transient components
+  // are specially handled, since they may or may not actually still
+  // be there when you go to restore state later (e.g., on the next request!)
   enum Transient { TRUE };
 }



Mime
View raw message