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 2011/02/24 16:27:11 UTC

svn commit: r1074187 - in /myfaces/trinidad/branches/1.2.12.5.0-branch/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal: renderkit/core/skin/ style/ style/cache/ style/xml/parse/

Author: jwaldman
Date: Thu Feb 24 15:27:09 2011
New Revision: 1074187

URL: http://svn.apache.org/viewvc?rev=1074187&view=rev
Log:
TRINIDAD-2025 move _styleNodeToStyleMap to FileSystemStyleCache to take advantage of same entires on the higher, browser or version or platform layer
TRINIDAD-2032 perf: save memory by sharing selector objects in filesystemstylecache
(This does not have the changes from 2025 for StyleSheetDocument, to use UnmodifiableStyle in the map instead of CSSStyle. It wasn't tested, so I did not want to backport it if it wasn't tested. It was a very small change anyway.)

Added:
    myfaces/trinidad/branches/1.2.12.5.0-branch/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/UnmodifiableStyle.java   (with props)
Removed:
    myfaces/trinidad/branches/1.2.12.5.0-branch/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/xml/parse/CSSStyleParser.java
Modified:
    myfaces/trinidad/branches/1.2.12.5.0-branch/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/renderkit/core/skin/XhtmlSkin.java
    myfaces/trinidad/branches/1.2.12.5.0-branch/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/BaseStyle.java
    myfaces/trinidad/branches/1.2.12.5.0-branch/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/CSSStyle.java
    myfaces/trinidad/branches/1.2.12.5.0-branch/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/cache/FileSystemStyleCache.java
    myfaces/trinidad/branches/1.2.12.5.0-branch/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/xml/parse/PropertyNode.java

Modified: myfaces/trinidad/branches/1.2.12.5.0-branch/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/renderkit/core/skin/XhtmlSkin.java
URL: http://svn.apache.org/viewvc/myfaces/trinidad/branches/1.2.12.5.0-branch/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/renderkit/core/skin/XhtmlSkin.java?rev=1074187&r1=1074186&r2=1074187&view=diff
==============================================================================
--- myfaces/trinidad/branches/1.2.12.5.0-branch/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/renderkit/core/skin/XhtmlSkin.java (original)
+++ myfaces/trinidad/branches/1.2.12.5.0-branch/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/renderkit/core/skin/XhtmlSkin.java Thu Feb 24 15:27:09 2011
@@ -18,17 +18,18 @@
  */
 package org.apache.myfaces.trinidadinternal.renderkit.core.skin;
 
-import java.util.HashMap;
 import java.util.Map;
 
+import java.util.concurrent.ConcurrentHashMap;
+
+import org.apache.myfaces.trinidad.style.Style;
 import org.apache.myfaces.trinidadinternal.renderkit.core.xhtml.SkinProperties;
 import org.apache.myfaces.trinidadinternal.renderkit.core.xhtml.SkinSelectors;
 import org.apache.myfaces.trinidadinternal.skin.icon.ContextImageIcon;
 import org.apache.myfaces.trinidadinternal.skin.icon.NullIcon;
 import org.apache.myfaces.trinidadinternal.skin.icon.ReferenceIcon;
 import org.apache.myfaces.trinidadinternal.skin.icon.TextIcon;
-import org.apache.myfaces.trinidadinternal.style.CSSStyle;
-
+import org.apache.myfaces.trinidadinternal.style.UnmodifiableStyle;
 
 
 /**
@@ -111,18 +112,20 @@ public class XhtmlSkin extends BaseSkin
   
   static
   {
-    // does this matter if it's not an ArrayMap?
-    _spinboxTopStyleMap = new HashMap<String, String>();
+    // todo Use ArrayMap instead of ConcurrentHashMap
+    // We were using CSSStyle instead of UnmodifiableStyle and that class copied the properties 
+    // into a ConcurrentHashMap. Changing this to another map will change the spinbox golden files.
+    _spinboxTopStyleMap = new ConcurrentHashMap<String, String>();
     _spinboxTopStyleMap.put("display", "block");
     // this is needed for the image
-    _spinboxBottomStyleMap = new HashMap<String, String>();
+    _spinboxBottomStyleMap = new ConcurrentHashMap<String, String>();
     _spinboxBottomStyleMap.put("display", "block");
     _spinboxBottomStyleMap.put("padding-top", "2px");
 
   }
   
-  private static final CSSStyle spinboxTopStyle = new CSSStyle(_spinboxTopStyleMap);
-  private static final CSSStyle spinboxBottomStyle = new CSSStyle(_spinboxBottomStyleMap);
+  private static final Style spinboxTopStyle = new UnmodifiableStyle(_spinboxTopStyleMap);
+  private static final Style spinboxBottomStyle = new UnmodifiableStyle(_spinboxBottomStyleMap);
 
 
 

Modified: myfaces/trinidad/branches/1.2.12.5.0-branch/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/BaseStyle.java
URL: http://svn.apache.org/viewvc/myfaces/trinidad/branches/1.2.12.5.0-branch/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/BaseStyle.java?rev=1074187&r1=1074186&r2=1074187&view=diff
==============================================================================
--- myfaces/trinidad/branches/1.2.12.5.0-branch/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/BaseStyle.java (original)
+++ myfaces/trinidad/branches/1.2.12.5.0-branch/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/BaseStyle.java Thu Feb 24 15:27:09 2011
@@ -30,6 +30,7 @@ import java.util.concurrent.ConcurrentHa
  * TODO Remove the ParsedProperty code from Trinidad. It is only used for
  * the un-used image generation code.
  * TODO Then remove CoreStyle and implement the public Style object instead.
+ * @deprecated
  * @version $Name:  $ ($Revision: adfrt/faces/adf-faces-impl/src/main/java/oracle/adfinternal/view/faces/style/BaseStyle.java#0 $) $Date: 10-nov-2005.18:57:54 $
  */
 abstract public class BaseStyle extends CoreStyle implements Serializable 

Modified: myfaces/trinidad/branches/1.2.12.5.0-branch/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/CSSStyle.java
URL: http://svn.apache.org/viewvc/myfaces/trinidad/branches/1.2.12.5.0-branch/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/CSSStyle.java?rev=1074187&r1=1074186&r2=1074187&view=diff
==============================================================================
--- myfaces/trinidad/branches/1.2.12.5.0-branch/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/CSSStyle.java (original)
+++ myfaces/trinidad/branches/1.2.12.5.0-branch/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/CSSStyle.java Thu Feb 24 15:27:09 2011
@@ -19,17 +19,15 @@
 package org.apache.myfaces.trinidadinternal.style;
 
 import java.util.Arrays;
-import java.util.Map;
 import java.util.Collections;
-import java.util.Iterator;
-
-import org.apache.myfaces.trinidad.style.Style;
+import java.util.Map;
 
 import org.apache.myfaces.trinidadinternal.style.util.CSSUtils;
 
 
 /**
- * Style implementation for CSS.
+ * Style implementation for CSS. Mutable.
+ * @deprecated Use UnmodifiableStyle which cannot be modified once it is created.
  *
  * @version $Name:  $ ($Revision: adfrt/faces/adf-faces-impl/src/main/java/oracle/adfinternal/view/faces/style/CSSStyle.java#0 $) $Date: 10-nov-2005.18:57:55 $
  */

Added: myfaces/trinidad/branches/1.2.12.5.0-branch/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/UnmodifiableStyle.java
URL: http://svn.apache.org/viewvc/myfaces/trinidad/branches/1.2.12.5.0-branch/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/UnmodifiableStyle.java?rev=1074187&view=auto
==============================================================================
--- myfaces/trinidad/branches/1.2.12.5.0-branch/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/UnmodifiableStyle.java (added)
+++ myfaces/trinidad/branches/1.2.12.5.0-branch/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/UnmodifiableStyle.java Thu Feb 24 15:27:09 2011
@@ -0,0 +1,131 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ * 
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ *  Unless required by applicable law or agreed to in writing,
+ *  software distributed under the License is distributed on an
+ *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ *  specific language governing permissions and limitations
+ *  under the License.
+ */
+package org.apache.myfaces.trinidadinternal.style;
+
+import java.util.Map;
+import java.util.Collections;
+
+import java.util.HashMap;
+import java.util.concurrent.ConcurrentHashMap;
+
+import org.apache.myfaces.trinidad.style.Style;
+
+/**
+ * Style implementation for CSS properties. Once created, you cannot mutate since there is no setProperties method.
+ * The toInlineString prints the properties out in CSS format, e.g., color: blue; font-size: 12px;
+ * To save memory, as the constructor parameter
+ * use an ArrayMap instead of a HashMap or ConcurrentHashMap if you can.
+ *
+ * @version $Name:  $ ($Revision: adfrt/faces/adf-faces-impl/src/main/java/oracle/adfinternal/view/faces/style/CSSStyle.java#0 $) $Date: 10-nov-2005.18:57:55 $
+ */
+public class UnmodifiableStyle extends Style
+{
+  /**
+   * Creates a Style object with the specified properties. There is no no-argument constructor
+   * because you must pass in all your properties when you create your object.
+   *
+   * @param properties The properties of this style, like  background-color, blue.  The
+   *   values must be Strings. 
+   * @throws NullPointerException if properties is null
+   */
+  public UnmodifiableStyle(Map<String, String> properties)
+  {
+    if (properties == null)
+      throw new NullPointerException("properties must be non-null");
+
+     _propertiesMap = Collections.unmodifiableMap(properties);
+  }
+     
+  @Override
+  public Map<String, String> getProperties()
+  {
+    return _propertiesMap;
+  }
+  
+  /**
+   * Converts the style to a String suitable for use as an inline style
+   * attribute value.
+   */
+  @Override
+  public String toInlineString()
+  {
+    String inline = _inline;
+
+    if (inline != null)
+      return inline;
+    
+    Map<String, String> properties = getProperties();
+    StringBuffer buffer = new StringBuffer(_DEFAULT_BUFFER_SIZE);
+    boolean first = true;   
+    
+    for (Map.Entry<String, String> entrySet : properties.entrySet())
+    {
+
+      if (first)
+        first = false;
+      else
+        buffer.append(";");
+      String name = entrySet.getKey();
+      String value = entrySet.getValue();
+      buffer.append(name);
+      buffer.append(":");
+      buffer.append(value);
+    }
+
+    inline = buffer.toString();
+    
+    _inline = inline;
+
+    return inline;
+  }
+
+  @Override
+  public String toString()
+  {
+    return "UnmodifiableStyle[css=" + toInlineString() + "]"; 
+  }
+  
+
+  @Override
+  public boolean equals(Object obj)
+  {
+    if (this == obj)
+      return true;
+    if (!(obj instanceof UnmodifiableStyle))
+      return false;
+
+    UnmodifiableStyle test = (UnmodifiableStyle)obj;
+    return this._propertiesMap.equals(test._propertiesMap);
+  }
+
+  @Override
+  public int hashCode()
+  {
+    return _propertiesMap.hashCode();
+  }
+
+  // The cached inline String value
+  // Marking it volatile guarantees that a read of _inline 
+  // always returns the most recent write by any thread.
+  transient volatile private String _inline;
+  final private Map<String, String> _propertiesMap;
+
+  // Default length for inline string buffer
+  private static final int _DEFAULT_BUFFER_SIZE = 100;
+}

Propchange: myfaces/trinidad/branches/1.2.12.5.0-branch/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/UnmodifiableStyle.java
------------------------------------------------------------------------------
    svn:eol-style = native

Modified: myfaces/trinidad/branches/1.2.12.5.0-branch/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/cache/FileSystemStyleCache.java
URL: http://svn.apache.org/viewvc/myfaces/trinidad/branches/1.2.12.5.0-branch/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/cache/FileSystemStyleCache.java?rev=1074187&r1=1074186&r2=1074187&view=diff
==============================================================================
--- myfaces/trinidad/branches/1.2.12.5.0-branch/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/cache/FileSystemStyleCache.java (original)
+++ myfaces/trinidad/branches/1.2.12.5.0-branch/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/cache/FileSystemStyleCache.java Thu Feb 24 15:27:09 2011
@@ -52,6 +52,7 @@ import org.apache.myfaces.trinidad.skin.
 import org.apache.myfaces.trinidad.style.Selector;
 import org.apache.myfaces.trinidad.style.Style;
 import org.apache.myfaces.trinidad.style.Styles;
+import org.apache.myfaces.trinidad.util.ArrayMap;
 import org.apache.myfaces.trinidad.util.CollectionUtils;
 import org.apache.myfaces.trinidadinternal.agent.TrinidadAgent;
 import org.apache.myfaces.trinidadinternal.renderkit.core.CoreRenderingContext;
@@ -62,9 +63,9 @@ import org.apache.myfaces.trinidadintern
 import org.apache.myfaces.trinidadinternal.share.io.DefaultNameResolver;
 import org.apache.myfaces.trinidadinternal.share.xml.JaxpXMLProvider;
 import org.apache.myfaces.trinidadinternal.share.xml.XMLProvider;
-import org.apache.myfaces.trinidadinternal.style.CSSStyle;
 import org.apache.myfaces.trinidadinternal.style.StyleContext;
 import org.apache.myfaces.trinidadinternal.style.StyleProvider;
+import org.apache.myfaces.trinidadinternal.style.UnmodifiableStyle;
 import org.apache.myfaces.trinidadinternal.style.util.CSSGenerationUtils;
 import org.apache.myfaces.trinidadinternal.style.util.NameUtils;
 import org.apache.myfaces.trinidadinternal.style.util.StyleWriterFactory;
@@ -81,7 +82,11 @@ import org.xml.sax.SAXException;
 
 /**
  * The FileSystemStyleCache is a StyleProvider implementation which
- * caches generated CSS style sheets on the file system.
+ * caches generated CSS style sheets on the file system. A FileSystemStyleCache object is for one Skin.
+ * The FileSystemStyleCache instance contains Entry objects for a Skin: one Entry
+ * object per unique generated CSS style sheet (e.g., gecko and ie will most
+ * likely have different generated CSS style sheets for the same Skin because these browsers tend to need
+ * different css rules).
  *
  * Note that StyleProviders are responsible for providing access
  * both to style information (eg. getStyleSheetURI(), getStyles()) as
@@ -399,6 +404,8 @@ public class FileSystemStyleCache implem
         _cache = null;
         _entryCache = null;
         _document = null;
+        _reusableStyleMap = null;
+        _reusableSelectorMap = null;
         _shortStyleClassMap = null;
         _namespacePrefixes  = null;
       }
@@ -414,6 +421,10 @@ public class FileSystemStyleCache implem
         _cache = new ConcurrentHashMap<Key, Entry>();
       if (_entryCache == null)
         _entryCache = new ConcurrentHashMap<Object, Entry>(19);
+      if (_reusableStyleMap == null)
+        _reusableStyleMap = new ConcurrentHashMap<Style, Style>();
+      if (_reusableSelectorMap == null)
+        _reusableSelectorMap = new ConcurrentHashMap<Selector, Selector>();
 
       cache = _cache;
       entryCache = _entryCache;
@@ -531,6 +542,34 @@ public class FileSystemStyleCache implem
     if (styleNodes == null)
       return null;
 
+    /* This code fills in the <Selector, Style> resolvedSelectorStyleMap map.
+     * We use _reusableStyleMap to reuse the Style objects when possible
+     * since we have a large number of Style objects. */
+    ConcurrentMap<Selector, Style> resolvedSelectorStyleMap = null;
+    for (int i=0; i < styleNodes.length; i++)
+    {
+      String selector = styleNodes[i].getSelector();
+      if (selector != null)
+      {
+        Style style = _convertStyleNodeToStyle(styleNodes[i], _reusableStyleMap);
+        if (resolvedSelectorStyleMap == null)
+          resolvedSelectorStyleMap = new ConcurrentHashMap<Selector, Style>();
+
+        // To save memory, we reuse Selector objects
+        Selector selectorCreated = Selector.createSelector(selector);
+        Selector cachedSelector = _reusableSelectorMap.get(selectorCreated);
+        if (cachedSelector == null)
+        {
+          _reusableSelectorMap.put(selectorCreated, selectorCreated);
+          resolvedSelectorStyleMap.put(selectorCreated, style);
+        }
+        else
+        {
+          resolvedSelectorStyleMap.put(cachedSelector, style);
+        }
+        
+      }
+    }
 
     // Generate the style sheet file, if it isn't already generated,
     // and return the uri.
@@ -560,8 +599,8 @@ public class FileSystemStyleCache implem
     // Create a new entry and cache it in the "normal" cache. The "normal" cache is one
     // where the key is the Key object which is built based on information from the StyleContext,
     // like browser, agent, locale, direction.
-    Styles styles = new StylesImpl(styleNodes, namespacePrefixes, _STYLE_KEY_MAP,
-                                   shortStyleClassMap,  _isCompressStyles(context));
+    Styles styles = new StylesImpl(namespacePrefixes, _STYLE_KEY_MAP,
+                                   shortStyleClassMap,  _isCompressStyles(context), resolvedSelectorStyleMap);
     Entry entry = new Entry(uris, styles, icons, skinProperties);
     cache.put(key, entry);
 
@@ -738,13 +777,13 @@ public class FileSystemStyleCache implem
               _LOG.warning(ex);
           }
         }
-        
+
         skinPropertiesMap.put(key, (propValueObj != null ? propValueObj : value));
 
       }
     }
     return skinPropertiesMap;
-    
+
   }
 
   /**
@@ -880,9 +919,9 @@ public class FileSystemStyleCache implem
         // I've seen the delete fail when we try to delete right after the file was created -
         // like if the skin css file is modified and the page refreshed immediately after the
         // app was initially run.
-        if (!success && _LOG.isWarning())
+        if (!success && _LOG.isInfo())
         {
-          _LOG.warning("COULD_NOT_DELETE_FILE", file.getName());
+          _LOG.info("COULD_NOT_DELETE_FILE", file.getName());
         }
       }
     }
@@ -983,9 +1022,8 @@ public class FileSystemStyleCache implem
     {
       // This might happen if we couldn't delete the css file that was already there, so we
       // are unable to recreate it.
-      if (_LOG.isWarning())
-        _LOG.warning("IOEXCEPTION_OPENNING_FILE", file);
-        _LOG.warning(e);
+      if (_LOG.isInfo())
+        _LOG.info("IOEXCEPTION_OPENNING_FILE", file);
     }
 
     return out;
@@ -1211,7 +1249,53 @@ public class FileSystemStyleCache implem
 
 
   /**
-   * Key class used for hashing style sheet URIs
+   * Given a StyleNode object, which is an internal API that denotes a Style object
+   * with additional information like includedSelectors, create a simple public
+   * Style object which will be used in the SelectorStyleMap. When this method is called,
+   * the StyleNode object is already resolved (included selectors have been merged in)
+   * so that all the css properties are there.
+   * @param styleNode
+   * @param reusableStyleMap A Map<Style, Style>. This is
+   *  used so that we can reuse Style objects in StylesImpl if they have the same list of style property
+   *  names and values.
+   * @return A Style object created from the information in the styleNode. We reuse
+   *  Style objects if the properties are the same.
+   */
+  public Style _convertStyleNodeToStyle(
+    StyleNode          styleNode,
+    Map<Style, Style>  reusableStyleMap)
+  {
+    // Add in the properties for the style; PropertyNode interns the 'name' and the most common 'value's.
+    Collection<PropertyNode> propertyNodeList = styleNode.getProperties();
+    Map<String, String> styleProperties = new ArrayMap<String, String>(propertyNodeList.size());
+
+    for (PropertyNode property : propertyNodeList)
+    {
+      String name = property.getName();
+      String value = property.getValue();
+      if (name != null && value != null)
+      {
+        styleProperties.put(name, value);
+      }
+    }
+
+    // To save memory, we reuse Style objects for each FileSystemStyleCache instance.
+    Style style = new UnmodifiableStyle(styleProperties);
+    Style cachedStyle = reusableStyleMap.get(style);
+    if (cachedStyle == null)
+    {
+      reusableStyleMap.put(style, style);
+      return style;
+    }
+    else
+    {
+      return cachedStyle;
+    }
+  }
+
+  /**
+   * Key class used for hashing style sheet URIs. This key for the Entry
+   * cache is dependent on the agent, locale, direction, etc.
    */
   private static class Key
   {
@@ -1324,7 +1408,10 @@ public class FileSystemStyleCache implem
   }
 
   /**
-   * Cache entry class
+   * Cache entry class.
+   * The FileSystemStyleCache instance contains Entry objects: one Entry
+   * object per unique generated CSS style sheet (e.g., gecko and ie will most
+   * likely have different generated CSS style sheets).
    */
   private static class Entry
   {
@@ -1347,9 +1434,9 @@ public class FileSystemStyleCache implem
   }
 
   /**
-   * A key object which is used to hash Entrys in the entry cache.  The key for the entry
+   * A key object which is used to hash Entrys in the entry cache.  This key for the Entry
    * cache is the style sheet derivation list - that is a list of StyleSheetNodes, sorted
-   * by specficity.
+   * by specficity. It's not dependent on the agent, locale, direction like the Key object is.
    */
   private static class DerivationKey
   {
@@ -1415,9 +1502,6 @@ public class FileSystemStyleCache implem
     private boolean _short;   // Do we use short style classes?
   }
 
-
-
-
   /**
    * A Styles implementation that adds the resolved (merged together based on the StyleContext)
    * StyleNodes to a Map. Only the style selectors and not the aliased (aka named) styles
@@ -1432,20 +1516,20 @@ public class FileSystemStyleCache implem
      * StyleNode are all null. This way we do not have to resolve the
      * styles based on the StyleContext when someone calls getStyles,
      * etc.
-     * @param resolvedStyles
      * @param namespacePrefixArray an array of namespace prefixes that are used in the custom css
      * skinning selectors, like "af" in af|inputText.
      * @param afSelectorMap a map from one selector to another (like af|panelHeader::link maps to
      * af|panelHeader A
      * @param shortStyleClassMap a map from the  non-compressed styleclass
      * to a compressed styleclass.
+     * @param resolvedSelectorStyleMap
      */
     public StylesImpl(
-        StyleNode[]         resolvedStyles,
         String[]            namespacePrefixArray,
         Map<String, String> afSelectorMap,
         Map<String, String> shortStyleClassMap,
-        boolean             compress
+        boolean             compress,
+        Map<Selector, Style> resolvedSelectorStyleMap
       )
     {
       // store these local variables to be used in getNativeSelectorString
@@ -1453,48 +1537,7 @@ public class FileSystemStyleCache implem
       _afSelectorMap = afSelectorMap;
       _shortStyleClassMap = shortStyleClassMap;
       _compress = compress;
-      // create a Selector->Style map right here (aggressively versus lazily)
-      ConcurrentMap<Selector, Style> resolvedSelectorStyleMap = null;
-      /* This is used so that we can re-use Style objects if the property name and value 
-       * is the same. Instead of creating a Style object for every property, we only
-       * create it for unique property name/values, and we store these in the map. 
-       * Then we pull them out of the map to create a Style object. */
-      Map<StyleKey, Style> styleNodeToStyleMap = new ConcurrentHashMap<StyleKey, Style>();
 
-      // Loop through each StyleNode and use it to add to the StyleMap.
-      for (int i=0; i < resolvedStyles.length; i++)
-      {
-        String selector = resolvedStyles[i].getSelector();
-        if (selector != null)
-        {
-          Style style = _convertStyleNodeToStyle(resolvedStyles[i], styleNodeToStyleMap);
-          if (resolvedSelectorStyleMap == null)
-            resolvedSelectorStyleMap = new ConcurrentHashMap<Selector, Style>();
-          resolvedSelectorStyleMap.put(Selector.createSelector(selector), style);
-        }
-        /*
-        else
-        {
-          // For now, do not add the named styles to the map. If we do add the named styles
-          // to the map then we should in SkinStyleSheetParserUtils put the full name in
-          // the StyleNode, not strip out the '.' or the ':alias'. However, in the XSS
-          // the named styles do not have the '.' or the ':alias' which is why we string them out.
-          // if we put them back, then things won't merge correctly. How do I workaround this?
-          // Do I change all the named styles in the XSS file?
-          // I think the best thing to do is to change the XSS file, since that is proprietary,
-          // and no one should be relying on it. If we instead kept stripping out the . and the alias
-          // and required the person to ask for the alias without this,
-          // that is much more confusing to the user.
-          String name = _resolvedStyles[i].getName();
-          if (name != null)
-          {
-            // create a Style Object from the StyleNode object
-            Style style = _convertStyleNode(resolvedStyles[i]);
-            resolvedSelectorStyleMap.put(name, style);
-          }
-        }
-        */
-      }
       if (resolvedSelectorStyleMap != null)
         _unmodifiableResolvedSelectorStyleMap =
           Collections.unmodifiableMap(resolvedSelectorStyleMap);
@@ -1550,110 +1593,9 @@ public class FileSystemStyleCache implem
     }
 
 
-    /**
-     * Given a StyleNode object, which is an internal API that denotes a Style object
-     * with additional information like includedSelectors, create a simple public
-     * Style object which will be used in the SelectorStyleMap. The StyleNode object
-     * should already be resolved (included selectors have been merged in)
-     * so that all the css properties are there.
-     * @param styleNode
-     * @param styleNodeToStyleMap A Map holding StyleKey objects to Style objects. This is 
-     *  used so that we can reuse CSSStyle objects if they have the same list of style property
-     *  names and values.
-     * @return A Style object created from the information in the styleNode. We reuse
-     *  Style objects if the properties are the same.
-     */
-    public Style _convertStyleNodeToStyle(
-      StyleNode            styleNode, 
-      Map<StyleKey, Style> styleNodeToStyleMap)
-    {
-      Map<String, String> styleProperties = new ConcurrentHashMap<String, String>();
-      // Add in the properties for the style
-      Iterable<PropertyNode> propertyNodeList = styleNode.getProperties();
-      for (PropertyNode property : propertyNodeList)
-      {
-        String name = property.getName();
-        String value = property.getValue();
-        // We get a NPE in CSSStyle(styleProperties) -> putAll if we add a value that is null.
-        // See the BaseStyle(Map<String, String> propertiesMap) constructor.
-        if (name != null && value != null)
-          styleProperties.put(name, value);
-      }
-
-      // To save memory, we reuse CSSStyle objects if 
-      // they have the same list of style property names and values.
-      // StyleKey is the key into the StyleKey, CSSStyle map.
-      StyleKey key = new StyleKey(styleProperties);
-      Style cachedStyle = styleNodeToStyleMap.get(key);
-      if (cachedStyle == null)
-      {
-        // no match is cached yet, so create a new CSSStyle and cache in the map.
-        Style style = new CSSStyle(styleProperties);
-        styleNodeToStyleMap.put(key, style);
-        return style;         
-      }
-      else
-      {
-        return cachedStyle;
-      }
-    }
-
-    /**
-     * A StyleKey object is used as a key into a map so that we can share CSSStyle objects
-     * if they are equal and they have the same hashCode.
-     */
-    private static class StyleKey
-    {
-      public StyleKey(Map<String, String> styleProperties)
-      {
-        _styleProperties = styleProperties;
-        _hashCode = _hashCode();
-      }
-
-      @Override
-      public int hashCode()
-      {
-        return _hashCode;
-      }
-
-      @Override  
-      public boolean equals(Object obj)
-      {
-        if (this == obj)
-          return true;
-        if (!(obj instanceof StyleKey))
-          return false;
-
-        // obj at this point must be a StyleKey
-        StyleKey test = (StyleKey)obj;
-        return test._styleProperties.equals(this._styleProperties);
-      }
-
-      /**
-       * Private implementation of hashCode. This way we can cache the hashcode.
-       * @return
-       */
-      private int _hashCode() 
-      {
-        int hash = 17;
-        // take each style property name and value and create a hashCode from it.
-        for (Map.Entry<String, String> e : _styleProperties.entrySet())
-        {
-          String name = e.getKey();
-          hash = 37*hash + ((null == name) ? 0 : name.hashCode());
-
-          String value = e.getValue();
-          hash = 37*hash + ((null == value) ? 0 : value.hashCode());
 
-        }
-        return hash;        
-      }
-      
-      private final Map<String, String> _styleProperties;
-      private final int _hashCode;
 
 
-    }
 
     private final Map<Selector, Style> _unmodifiableResolvedSelectorStyleMap;
     private final Map<String, String>  _afSelectorMap;
@@ -1722,6 +1664,17 @@ public class FileSystemStyleCache implem
   /** The parsed StyleSheetDocument */
   private StyleSheetDocument _document;
 
+  /** Since each FileSystemStyleCache$Entry object holds a FileSystemStyleCache$StylesImpl object
+   *  which holds on to many Style objects, it reduces the memory consumption by about half
+   *  if we reuse Style objects per FileSystemStyleCache instance rather than per
+   *  FileSystemStyleCache$Entry instance. The is the map we use to store unique Style objects. */
+  private ConcurrentMap<Style, Style> _reusableStyleMap;
+
+  /** Use this to store Selector objects so that they can be reused in all the FileSystemStyleCache$StylesImpl
+   * objects. A generated css file can contain 4533 selectors at 16 bytes each. The Selectors will largely
+   * be the same between FileSystemStyleCache$StylesImpl instances, so they should be shared. */
+  private ConcurrentMap<Selector, Selector> _reusableSelectorMap;
+  
   /** The cache of style sheet URIs */
   private ConcurrentMap<Key, Entry> _cache;
 

Modified: myfaces/trinidad/branches/1.2.12.5.0-branch/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/xml/parse/PropertyNode.java
URL: http://svn.apache.org/viewvc/myfaces/trinidad/branches/1.2.12.5.0-branch/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/xml/parse/PropertyNode.java?rev=1074187&r1=1074186&r2=1074187&view=diff
==============================================================================
--- myfaces/trinidad/branches/1.2.12.5.0-branch/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/xml/parse/PropertyNode.java (original)
+++ myfaces/trinidad/branches/1.2.12.5.0-branch/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/xml/parse/PropertyNode.java Thu Feb 24 15:27:09 2011
@@ -16,6 +16,12 @@
 
 package org.apache.myfaces.trinidadinternal.style.xml.parse;
 
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+
 import org.apache.myfaces.trinidad.logging.TrinidadLogger;
 /**
  * Private implementation of PropertyNode.
@@ -24,7 +30,6 @@ import org.apache.myfaces.trinidad.loggi
  */
 public class PropertyNode
 {
-
   /**
    * Creates a PropertyNode with the specified name and value.
    * @param name name of the propertyNode. Examples are 'font-size', 'background-color'.
@@ -42,6 +47,24 @@ public class PropertyNode
     // like color, background-color, background-image, font-size, etc.
     // This will improve the memory used.
     _name = name.intern();
+     
+      if (value != null)
+      {
+      if (_INTERN_VALUES_FOR.contains(name))
+      {
+        value = value.intern();
+      }
+      else
+      {
+        String internedValue =  _INTERNED_VALUES.get(value);
+       
+        if (internedValue != null)
+        {
+          value = internedValue;
+        }
+      }
+      }
+     
     _value = value;
   }
 
@@ -94,6 +117,107 @@ public class PropertyNode
       "value=" + _value + "]";
   }
 
+  // map to the interned value for values that are common enough that we should intern them,
+  // but we can't always intern based purely on the property name.  A good example are
+  // proprties like vertical-align or cursor where there are many fixed values but the
+  // option to use a percentage or URL is also there
+  private static final Map<String, String> _INTERNED_VALUES = new HashMap<String, String>();
+
+  static
+  {
+    // initialize values that we should share
+    String[] internedValues = new String[]
+    {
+      "#000000",
+      "#FFFFFF",
+      "#ffffff",
+      "0",
+      "0%",
+      "0px",
+      "1",
+      "10%",
+      "100%",
+      "1px",
+      "1em",
+      "2",
+      "2px",
+      "3px",
+      "auto",
+      "4px",
+      "5px",
+      "50%",
+      "6px",
+      "7px",
+      "8px",
+      "9px",
+      "auto",
+      "baseline",
+      "black",
+      "bottom",
+      "center",
+      "center center",
+      "crosshair",
+      "default",
+      "e-resize",
+      "gray",
+      "help",
+      "inherit",
+      "left",
+      "middle",
+      "move",
+      "n-resize",
+      "ne-resize",
+      "nw-resize",
+      "none",
+      "pointer",
+      "progress",
+      "right",
+      "s-resize",
+      "se-resize",
+      "sub",
+      "super",
+      "sw-resize",
+      "text",
+      "text-bottom",
+      "text-top",
+      "top",
+      "transparent",
+      "w-resize",
+      "wait",
+      "white"
+    };
+   
+    for (int i = 0; i < internedValues.length; i++)
+    {
+      String internedValue = internedValues[i];
+     
+      _INTERNED_VALUES.put(internedValue, internedValue);
+    }
+  }
+ 
+  // property names that we should always intern the values for because they are either
+  // enumerations or are repeated often (font-family)
+  private static final Set<String> _INTERN_VALUES_FOR = new HashSet<String>(
+    Arrays.asList("-moz-box-sizing",
+                  "background-repeat",
+                  "border-style",
+                  "display",
+                  "float",
+                  "font-family",
+                  "font-style",
+                  "font-weight",
+                  "list-style-position",
+                  "list-style-type",
+                  "overflow",
+                  "overflow-x",
+                  "overflow-y",
+                  "position",
+                  "text-align",
+                  "text-decoration",
+                  "transparent",
+                  "visibility",
+                  "white-space"));
+
   private final String _name;
   private final String _value;
   private static final TrinidadLogger _LOG = TrinidadLogger.createTrinidadLogger(