You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@myfaces.apache.org by jw...@apache.org on 2007/10/03 04:07:13 UTC

svn commit: r581474 - in /myfaces/trinidad/branches/jwaldman1.2.2temp-branch/trinidad: trinidad-api/src/main/java/org/apache/myfaces/trinidad/component/ trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/xml/parse/

Author: jwaldman
Date: Tue Oct  2 19:07:12 2007
New Revision: 581474

URL: http://svn.apache.org/viewvc?rev=581474&view=rev
Log:
merge in 
TRINIDAD-630 Table stamp state could be greatly optimized for size
and TRINIDAD-633: StyleNode objects could be much more lightweight
so that we can test these performance enhancements.
branch jwaldman1.2.2temp-branch

Modified:
    myfaces/trinidad/branches/jwaldman1.2.2temp-branch/trinidad/trinidad-api/src/main/java/org/apache/myfaces/trinidad/component/StampState.java
    myfaces/trinidad/branches/jwaldman1.2.2temp-branch/trinidad/trinidad-api/src/main/java/org/apache/myfaces/trinidad/component/UIXCollection.java
    myfaces/trinidad/branches/jwaldman1.2.2temp-branch/trinidad/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/xml/parse/StyleNode.java

Modified: myfaces/trinidad/branches/jwaldman1.2.2temp-branch/trinidad/trinidad-api/src/main/java/org/apache/myfaces/trinidad/component/StampState.java
URL: http://svn.apache.org/viewvc/myfaces/trinidad/branches/jwaldman1.2.2temp-branch/trinidad/trinidad-api/src/main/java/org/apache/myfaces/trinidad/component/StampState.java?rev=581474&r1=581473&r2=581474&view=diff
==============================================================================
--- myfaces/trinidad/branches/jwaldman1.2.2temp-branch/trinidad/trinidad-api/src/main/java/org/apache/myfaces/trinidad/component/StampState.java (original)
+++ myfaces/trinidad/branches/jwaldman1.2.2temp-branch/trinidad/trinidad-api/src/main/java/org/apache/myfaces/trinidad/component/StampState.java Tue Oct  2 19:07:12 2007
@@ -69,12 +69,21 @@
     Map<DualKey, Object> comparant = Collections.emptyMap();
     if (_rows == comparant)
     {
+      if (value == null)
+        return;
+
       // =-=AEW Better default sizes
       _rows = new HashMap<DualKey, Object>(109);
     }
 
     DualKey dk = new DualKey(currencyObj, key);
-    _rows.put(dk, value);
+    // Make sure that if we're applying a null value, that we 
+    // don't hold on to the key and retain the entry - just nuke
+    // the entry
+    if (value == null)
+      _rows.remove(dk);
+    else
+      _rows.put(dk, value);
   }
 
   public int size()
@@ -94,8 +103,6 @@
   public static Object saveStampState(FacesContext context, UIComponent stamp)
   {
     RowState state = _createState(stamp);
-    if (state != null)
-      state.saveRowState(stamp);
     return state;
   }
 
@@ -119,45 +126,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 +283,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 +330,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 +373,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/branches/jwaldman1.2.2temp-branch/trinidad/trinidad-api/src/main/java/org/apache/myfaces/trinidad/component/UIXCollection.java
URL: http://svn.apache.org/viewvc/myfaces/trinidad/branches/jwaldman1.2.2temp-branch/trinidad/trinidad-api/src/main/java/org/apache/myfaces/trinidad/component/UIXCollection.java?rev=581474&r1=581473&r2=581474&view=diff
==============================================================================
--- myfaces/trinidad/branches/jwaldman1.2.2temp-branch/trinidad/trinidad-api/src/main/java/org/apache/myfaces/trinidad/component/UIXCollection.java (original)
+++ myfaces/trinidad/branches/jwaldman1.2.2temp-branch/trinidad/trinidad-api/src/main/java/org/apache/myfaces/trinidad/component/UIXCollection.java Tue Oct  2 19:07:12 2007
@@ -782,58 +782,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;
   }
 
@@ -847,58 +875,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]);
+    }
   }
 
   /**
@@ -1255,11 +1277,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);
     }
@@ -1463,8 +1486,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 };
 }

Modified: myfaces/trinidad/branches/jwaldman1.2.2temp-branch/trinidad/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/xml/parse/StyleNode.java
URL: http://svn.apache.org/viewvc/myfaces/trinidad/branches/jwaldman1.2.2temp-branch/trinidad/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/xml/parse/StyleNode.java?rev=581474&r1=581473&r2=581474&view=diff
==============================================================================
--- myfaces/trinidad/branches/jwaldman1.2.2temp-branch/trinidad/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/xml/parse/StyleNode.java (original)
+++ myfaces/trinidad/branches/jwaldman1.2.2temp-branch/trinidad/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/xml/parse/StyleNode.java Tue Oct  2 19:07:12 2007
@@ -74,28 +74,40 @@
     _selector = selector;
     _resetProperties = resetProperties;
 
+
+    // Structure copying:
+    // We have quite a lot of StyleNodes floating around;  so making
+    // them lightweight is highly desirable.  Consequently, use
+    // the lightest weight lists possible:  emptyList() if null
+    // or an empty array, and singletonList() if it's a one-element array
+
     // Initialize _properties
     // ------------------------------
-    if (properties != null)
-      _properties = Collections.unmodifiableList(Arrays.asList(properties));
-    else
+    if (properties == null || (properties.length == 0))
       _properties = Collections.emptyList();
-
+    else if (properties.length == 1)
+      _properties = Collections.singletonList(properties[0]);
+    else
+      _properties = Collections.unmodifiableList(Arrays.asList(properties));
 
     // Initialize _includedStyles
     // ------------------------------
-    if (includedStyles != null)
-      _includedStyles = Collections.unmodifiableList(Arrays.asList(includedStyles));
-    else
+    if ((includedStyles == null) || (includedStyles.length == 0))
       _includedStyles = Collections.emptyList();
+    else if (includedStyles.length == 1)
+      _includedStyles = Collections.singletonList(includedStyles[0]);
+    else
+      _includedStyles = Collections.unmodifiableList(Arrays.asList(includedStyles));
 
 
     // Initialize _includedProperties
     // ------------------------------
-    if (includedProperties != null)
-      _includedProperties = Collections.unmodifiableList(Arrays.asList(includedProperties));
-    else
+    if ((includedProperties == null) || (includedProperties.length == 0))
       _includedProperties = Collections.emptyList();
+    else if (includedProperties.length == 1)
+      _includedProperties = Collections.singletonList(includedProperties[0]);
+    else
+      _includedProperties = Collections.unmodifiableList(Arrays.asList(includedProperties));
       
       
     // Initialize _inhibitAll and _inhibitedProperties
@@ -103,7 +115,7 @@
     boolean inhibitAll = false;
     // Convert inhibitedProperties Set to an unmodifiableList
 
-    if(inhibitedProperties != null)
+    if ((inhibitedProperties != null) && !inhibitedProperties.isEmpty())
     {
       List<String> inhibitedPropertiesList = new ArrayList<String>(inhibitedProperties.size());
       for(String property : inhibitedProperties)
@@ -135,7 +147,10 @@
         }
       }
 
-      _inhibitedProperties = Collections.unmodifiableList(inhibitedPropertiesList);
+      if (inhibitedPropertiesList.size() == 1)
+        _inhibitedProperties = Collections.singletonList(inhibitedPropertiesList.get(0));
+      else
+        _inhibitedProperties = Collections.unmodifiableList(inhibitedPropertiesList);
 
     }
     else