I've attached the change bundle. This implements solution 2, however,
to get around the main problem with that solution
the rendering optimization will only be done for components in the
default packages (javax.faces.component and javax.faces.component.html).
So any custom components that rely on the RI renderers will continue to
work as they did before.
SECTION: Modified Files
----------------------------
M jsf-api/src/javax/faces/component/UIComponent.java
- added package private List<String> to contain the names
of attributes/properties that have been added
- added a convenience method to obtain/init the list from bullet 1.
- update setValueExpression() to add/remove the name of the binding
to/from the list
M jsf-api/src/javax/faces/component/UIComponentBase.java
- Update the AttributesMap implementation's get,put,remove,putAll implementations
to interact with the setAttributes List as appropriate
M jsf-ri/src/com/sun/faces/renderkit/RenderKitUtils.java
- added canBeOptimized() method which checks the package of the
provided component. If the component is in the standard packages
(javax.faces.component/javax.faces.component.html) then we'll
use the optimized version of renderPassThruAttributes. If the component
isn't in the standard packages (this means a custom component leveraging
our renderers), then default back to the old style where we query
on all of the known pass through attributes.
- the optimized version of renderPassThruAttributes will sort
the setAttributes list, and then perform a binary search against
the known attributes. If found and the value isn't null, render
the attribute.
M jsf-tools/src/com/sun/faces/generate/HtmlComponentGenerator.java
- update the HtmlComponentGenerator to enchance the setters for those
attributes that correspond to pass through attributes. When they are
called, they will add the attribute to the setAttributes list.
SECTION: Diffs
----------------------------
Index: jsf-api/src/javax/faces/component/UIComponent.java
===================================================================
RCS file: /cvs/javaserverfaces-sources/jsf-api/src/javax/faces/component/UIComponent.java,v
retrieving revision 1.150
diff -u -r1.150 UIComponent.java
--- jsf-api/src/javax/faces/component/UIComponent.java 27 Apr 2007 22:00:04 -0000 1.150
+++ jsf-api/src/javax/faces/component/UIComponent.java 4 Sep 2007 17:52:34 -0000
@@ -41,6 +41,13 @@
package javax.faces.component;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+
import javax.el.ELContext;
import javax.el.ELException;
import javax.el.ValueExpression;
@@ -52,12 +59,6 @@
import javax.faces.event.FacesListener;
import javax.faces.render.Renderer;
-import java.io.IOException;
-import java.util.HashMap;
-import java.util.Iterator;
-import java.util.List;
-import java.util.Map;
-
/**
* <p><strong>UIComponent</strong> is the base class for all user interface
* components in JavaServer Faces. The set of {_at_link UIComponent} instances
@@ -77,6 +78,8 @@
public abstract class UIComponent implements StateHolder {
+ List<String> setAttributes;
+
// -------------------------------------------------------------- Attributes
@@ -254,6 +257,10 @@
//noinspection CollectionWithoutInitialCapacity
bindings = new HashMap<String, ValueExpression>();
}
+ List<String> sProperties = getSetAttributesList();
+ if (!sProperties.contains(name)) {
+ sProperties.add(name);
+ }
bindings.put(name, binding);
} else {
ELContext context =
@@ -266,6 +273,9 @@
}
} else {
if (bindings != null) {
+ List<String> sProperties = getSetAttributesList();
+ sProperties.remove(name);
+ setAttributes.remove(name);
bindings.remove(name);
if (bindings.isEmpty()) {
bindings = null;
@@ -1139,4 +1149,14 @@
protected abstract Renderer getRenderer(FacesContext context);
+ // --------------------------------------------------------- Package Private
+
+
+ List<String> getSetAttributesList() {
+ if (setAttributes == null) {
+ setAttributes = new ArrayList<String>(6);
+ }
+ return setAttributes;
+ }
+
}
Index: jsf-api/src/javax/faces/component/UIComponentBase.java
===================================================================
RCS file: /cvs/javaserverfaces-sources/jsf-api/src/javax/faces/component/UIComponentBase.java,v
retrieving revision 1.153.4.1
diff -u -r1.153.4.1 UIComponentBase.java
--- jsf-api/src/javax/faces/component/UIComponentBase.java 28 Aug 2007 22:57:27 -0000 1.153.4.1
+++ jsf-api/src/javax/faces/component/UIComponentBase.java 4 Sep 2007 17:52:34 -0000
@@ -1242,7 +1242,7 @@
public Object saveState(FacesContext context) {
if (values == null) {
- values = new Object[8];
+ values = new Object[9];
}
if (attributes != null) {
@@ -1258,6 +1258,7 @@
values[5] = renderedSet ? Boolean.TRUE : Boolean.FALSE;
values[6] = rendererType;
values[7] = saveAttachedState(context, listeners);
+ values[8] = setAttributes;
assert(!transientFlag);
return (values);
@@ -1294,6 +1295,7 @@
listeners = restoredListeners;
}
}
+ setAttributes = (List<String>) values[8];
}
@@ -1511,6 +1513,9 @@
// private 'attributes' map directly to the state saving process.
private static class AttributesMap implements Map<String, Object>, Serializable {
+ private static final String KEY =
+ UIComponentBase.class.getName() + ".setAttributes";
+
private HashMap<String, Object> attributes;
private transient Map<String,PropertyDescriptor> pdMap;
private transient UIComponent component;
@@ -1519,8 +1524,10 @@
// -------------------------------------------------------- Constructors
private AttributesMap(UIComponent component) {
+
this.component = component;
this.pdMap = ((UIComponentBase) component).getDescriptorMap();
+
}
private AttributesMap(UIComponent component,
@@ -1530,6 +1537,9 @@
}
public boolean containsKey(Object keyObj) {
+ if (KEY.equals(keyObj)) {
+ return true;
+ }
String key = (String) keyObj;
PropertyDescriptor pd =
getPropertyDescriptor(key);
@@ -1549,6 +1559,9 @@
if (key == null) {
throw new NullPointerException();
}
+ if (KEY.equals(key)) {
+ return getSetAttributes(false);
+ }
PropertyDescriptor pd =
getPropertyDescriptor(key);
if (pd != null) {
@@ -1588,6 +1601,16 @@
throw new NullPointerException();
}
+ if (KEY.equals(keyValue)) {
+ if (component.setAttributes == null) {
+ if (value instanceof List) {
+ component.setAttributes = (List<String>) value;
+ return component.setAttributes;
+ }
+ }
+ return null;
+ }
+
PropertyDescriptor pd =
getPropertyDescriptor(keyValue);
if (pd != null) {
@@ -1620,11 +1643,15 @@
if (attributes == null) {
initMap();
}
+ List<String> sProperties = getSetAttributes(true);
+ if (!sProperties.contains(keyValue)) {
+ sProperties.add(keyValue);
+ }
return (attributes.put(keyValue, value));
}
}
- public void putAll(Map<? extends String, ? extends Object> map) {
+ public void putAll(Map<? extends String, ?> map) {
if (map == null) {
throw new NullPointerException();
}
@@ -1632,6 +1659,12 @@
if (attributes == null) {
initMap();
}
+ for (String key : map.keySet()) {
+ List<String> sProperties = getSetAttributes(true);
+ if (!sProperties.contains(key)) {
+ sProperties.add(key);
+ }
+ }
attributes.putAll(map);
}
@@ -1640,12 +1673,19 @@
if (key == null) {
throw new NullPointerException();
}
+ if (KEY.equals(key)) {
+ return null;
+ }
PropertyDescriptor pd =
getPropertyDescriptor(key);
if (pd != null) {
throw new IllegalArgumentException(key);
} else {
if (attributes != null) {
+ List<String> sProperties = getSetAttributes(false);
+ if (sProperties != null) {
+ sProperties.remove(key);
+ }
return (attributes.remove(key));
} else {
return null;
@@ -1740,6 +1780,14 @@
return h;
}
+ public List<String> getSetAttributes(boolean create) {
+ if (create) {
+ return component.getSetAttributesList();
+ } else {
+ return component.setAttributes;
+ }
+ }
+
private void initMap() {
attributes = new HashMap<String,Object>(8);
}
Index: jsf-ri/src/com/sun/faces/renderkit/RenderKitUtils.java
===================================================================
RCS file: /cvs/javaserverfaces-sources/jsf-ri/src/com/sun/faces/renderkit/RenderKitUtils.java,v
retrieving revision 1.43
diff -u -r1.43 RenderKitUtils.java
--- jsf-ri/src/com/sun/faces/renderkit/RenderKitUtils.java 19 Jul 2007 15:32:45 -0000 1.43
+++ jsf-ri/src/com/sun/faces/renderkit/RenderKitUtils.java 4 Sep 2007 17:52:34 -0000
@@ -36,27 +36,6 @@
package com.sun.faces.renderkit;
-import com.sun.faces.RIConstants;
-import com.sun.faces.config.WebConfiguration;
-import static com.sun.faces.config.WebConfiguration.BooleanWebContextInitParameter;
-import com.sun.faces.renderkit.html_basic.HtmlBasicRenderer.Param;
-import com.sun.faces.util.MessageUtils;
-import com.sun.faces.util.Util;
-import com.sun.faces.util.FacesLogger;
-
-import javax.faces.FacesException;
-import javax.faces.FactoryFinder;
-import javax.faces.component.UIComponent;
-import javax.faces.component.UISelectItem;
-import javax.faces.component.UISelectItems;
-import javax.faces.context.ExternalContext;
-import javax.faces.context.FacesContext;
-import javax.faces.context.ResponseWriter;
-import javax.faces.model.SelectItem;
-import javax.faces.render.RenderKit;
-import javax.faces.render.RenderKitFactory;
-import javax.faces.render.ResponseStateManager;
-
import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStream;
@@ -69,12 +48,35 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
+import java.util.Collections;
import java.util.Iterator;
+import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.logging.Level;
import java.util.logging.Logger;
+import javax.faces.FacesException;
+import javax.faces.FactoryFinder;
+import javax.faces.component.UIComponent;
+import javax.faces.component.UISelectItem;
+import javax.faces.component.UISelectItems;
+import javax.faces.context.ExternalContext;
+import javax.faces.context.FacesContext;
+import javax.faces.context.ResponseWriter;
+import javax.faces.model.SelectItem;
+import javax.faces.render.RenderKit;
+import javax.faces.render.RenderKitFactory;
+import javax.faces.render.ResponseStateManager;
+
+import com.sun.faces.RIConstants;
+import com.sun.faces.config.WebConfiguration;
+import static com.sun.faces.config.WebConfiguration.BooleanWebContextInitParameter;
+import com.sun.faces.renderkit.html_basic.HtmlBasicRenderer.Param;
+import com.sun.faces.util.FacesLogger;
+import com.sun.faces.util.MessageUtils;
+import com.sun.faces.util.Util;
+
/**
* <p>A set of utilities for use in {_at_link RenderKit}s.</p>
*/
@@ -146,7 +148,13 @@
* <p>JavaScript to be rendered when a commandLink is used.
* This may be expaned to include other uses.</p>
*/
- private static final String SUN_JSF_JS = RIConstants.FACES_PREFIX + "sunJsfJs";
+ private static final String SUN_JSF_JS = RIConstants.FACES_PREFIX + "sunJsfJs";
+
+
+ private static final String[] OPTIMIZED_PACKAGES = {
+ "javax.faces.component",
+ "javax.faces.component.html"
+ };
protected static final Logger LOGGER = FacesLogger.RENDERKIT.getLogger();
@@ -272,6 +280,10 @@
list.add((SelectItem) value);
} else if (value instanceof SelectItem[]) {
SelectItem[] items = (SelectItem[]) value;
+ // we manually copy the elements so that the list is
+ // modifiable. Arrays.asList() returns a non-mutable
+ // list.
+ //noinspection ManualArrayToCollectionCopy
for (SelectItem item : items) {
list.add(item);
}
@@ -336,16 +348,32 @@
assert (null != component);
Map<String, Object> attrMap = component.getAttributes();
-
- boolean isXhtml = writer.getContentType().equals(RIConstants.XHTML_CONTENT_TYPE);
- for (String attrName : attributes) {
- Object value =
- attrMap.get(attrName);
- if (value != null && shouldRenderAttribute(value)) {
- writer.writeAttribute(prefixAttribute(attrName, isXhtml),
- value,
- attrName);
+ // PENDING - think anyone would run the RI using another implementation
+ // of the jsf-api? If they did, then this would fall apart. That
+ // scenario seems extremely unlikely.
+ if (canBeOptimized(component)) {
+ //noinspection unchecked
+ List<String> setAttributes = (List<String>)
+ component.getAttributes().get("javax.faces.component.UIComponentBase.setAttributes");
+ if (setAttributes != null) {
+ renderPassThruAttributesOptimized(writer,
+ component,
+ attributes,
+ setAttributes);
+ }
+ } else {
+ boolean isXhtml = writer.getContentType()
+ .equals(RIConstants.XHTML_CONTENT_TYPE);
+ for (String attrName : attributes) {
+
+ Object value =
+ attrMap.get(attrName);
+ if (value != null && shouldRenderAttribute(value)) {
+ writer.writeAttribute(prefixAttribute(attrName, isXhtml),
+ value,
+ attrName);
+ }
}
}
@@ -457,6 +485,39 @@
// --------------------------------------------------------- Private Methods
+ private static boolean canBeOptimized(UIComponent component) {
+
+ String pkg = component.getClass().getPackage().getName();
+ return (Arrays.binarySearch(OPTIMIZED_PACKAGES, pkg) >= 0);
+
+ }
+
+
+ private static void renderPassThruAttributesOptimized(ResponseWriter writer,
+ UIComponent component,
+ String[] knownAttributes,
+ List<String> setAttributes)
+ throws IOException {
+
+ Collections.sort(setAttributes);
+ boolean isXhtml = writer.getContentType()
+ .equals(RIConstants.XHTML_CONTENT_TYPE);
+ Map<String, Object> attrMap = component.getAttributes();
+ for (String name : setAttributes) {
+
+ if (Arrays.binarySearch(knownAttributes, name) >= 0) {
+ Object value =
+ attrMap.get(name);
+ if (value != null && shouldRenderAttribute(value)) {
+ writer.writeAttribute(prefixAttribute(name, isXhtml),
+ value,
+ name);
+ }
+ }
+ }
+ }
+
+
/**
* <p>Determines if an attribute should be rendered based on the
* specified #attributeVal.</p>
Index: jsf-tools/src/com/sun/faces/generate/HtmlComponentGenerator.java
===================================================================
RCS file: /cvs/javaserverfaces-sources/jsf-tools/src/com/sun/faces/generate/HtmlComponentGenerator.java,v
retrieving revision 1.27
diff -u -r1.27 HtmlComponentGenerator.java
--- jsf-tools/src/com/sun/faces/generate/HtmlComponentGenerator.java 30 May 2007 17:05:05 -0000 1.27
+++ jsf-tools/src/com/sun/faces/generate/HtmlComponentGenerator.java 4 Sep 2007 17:52:35 -0000
@@ -259,6 +259,15 @@
writer.write("\");\n");
}
+ PropertyBean[] pbs = cb.getProperties();
+ for (PropertyBean pb : pbs) {
+ if (pb.isPassThrough() && pb.getDefaultValue() != null) {
+ writer.fwrite("getSetAttributes().add(\"");
+ writer.write(pb.getPropertyName());
+ writer.write("\");\n");
+ }
+ }
+
writer.outdent();
writer.fwrite("}\n\n\n");
@@ -426,6 +435,20 @@
writer.write(var);
writer.write("_set = true;\n");
}
+ if ((pb.isPassThrough() && pb.getDefaultValue() == null)
+ || (cb.getComponentClass().contains("HtmlCommandButton")
+ && "onclick".equals(pb.getPropertyName()))) {
+ writer.fwrite("List<String> setAttrs = getSetAttributes();\n");
+ writer.fwrite("if (setAttrs != null && !setAttrs.contains(\"");
+ writer.write(var);
+ writer.write("\")) {\n");
+ writer.indent();
+ writer.fwrite("getSetAttributes().add(\"");
+ writer.write(pb.getPropertyName());
+ writer.write("\");\n");
+ writer.outdent();
+ writer.fwrite("}\n");
+ }
writer.outdent();
writer.fwrite("}\n\n");
@@ -539,6 +562,20 @@
writer.outdent();
writer.fwrite("}\n\n\n");
+ writer.fwrite("private List<String> getSetAttributes() {\n");
+ writer.indent();
+ writer.fwrite("List<String> setAttributes = (List<String>) this.getAttributes().get(\"javax.faces.component.UIComponentBase.setAttributes\");\n");
+ writer.fwrite("if (setAttributes == null) {\n");
+ writer.indent();
+ writer.fwrite("setAttributes = new ArrayList<String>(4);\n");
+ writer.fwrite("this.getAttributes().put(\"javax.faces.component.UIComponentBase.setAttributes\", setAttributes);\n");
+ writer.outdent();
+ writer.fwrite("}\n");
+ writer.fwrite("return setAttributes;\n");
+ writer.outdent();
+ writer.fwrite("}\n\n");
+
+
// Generate the ending of this class
writer.outdent();
writer.write("}\n");
attached mail follows:
I think I have a solution to this problem (improves on solution 2, but
doesn't add the complexity from
solution 3).
I'll prototype over the weekend and post a change bundle early next week.
Ryan Lubke wrote:
> PROBLEM:
>
> In our current implementation, each renderer has an array of known
> passthrough
> attributes it needs to handle. When the component is rendered,
> this array is passed
> to RenderKitUtils.renderPassThruAttributes(). The logic here is
> pretty simple, we
> iterate through all of the known attributes and call
> component.getAttributes(). If we
> have a non-null return, we render the attribute.
>
> So as a simple example using the UIData.jsp example in the
> jsf-standard demo,
> we spend about 1872 ms over 1461 invocations. Most of this time is
> spent (1148ms)
> calling Method instances since most, if not all, of the
> pass-through attributes on the
> Html components map to java properties.
> I think in general it would be safe to say that it's not the norm
> for a component to have
> more than 6 pass-through attributes defined, so if a component has
> a total of 20 pass through
> attributes, then we waste a lot of cycles when only 6 or so need to
> be rendered.
>
> --------------------------------------------------------------------------
>
> SOLUTION 1:
> - Add a package-private List<String> to UIComponent with a method
> to will create/return the
> list. This list would be stored within the UIComponent
> instance's attribute map.
> - Update the implementation for UIComponent.setValueExpression()
> and the attribute map
> implementation to add attribute names to this list.
> - When rendering, check for the existence of this List. If
> present, then call an optimized
> version of RenderKitUtils.renderPassThroughAttributes() which
> will, for each of the
> attribute names in the List, search the known pass-through
> attributes for this component
> and render them.
>
> PROBLEMS WITH SOLUTION 1:
> - what if a developer uses a component binding where they create
> the component,
> call a setter (which maps to a pass-through attribute) against
> the component directly,
> and then call getAttributes().put(), or set a value expression?
> This would cause the
> list to be created which would trigger the optimized rendering
> path, but the value set
> via setter wouldn't be known, and thus wouldn't be rendered.
> --------------------------------------------------------------------------
>
>
> --------------------------------------------------------------------------
>
> SOLUTION 2:
> - Add a package-private List<String> to UIComponent with a
> method to will create/return the
> list. This list would be stored within the UIComponent
> instance's attribute map.
> - Update the implementation for
> UIComponent.setValueExpression() and the attribute map
> implementation to add attribute names to this list.
> - Update the HTML component generator to:
> - for each setter that maps to a pass-through attribute, ensure
> it will add the attribute name to the list.
> - for each pass-through attribute of the component that has a
> default property (such a HtmlForm's enctype),
> ensure the attribute is added to the list when the component
> is instantiated.
>
> PROBLEMS WITH SOLUTION 2:
> - The main problem with solution 1 is solved, but now, what if a
> developer extends
> the default HTML component and overrides a method mapping to a
> pass-through
> attribute and never calls super()? Some other causes the
> attribute list to be created,
> but because how the developer overrode the method, that
> attribute isn't in the list and
> therefore doesn't get rendered.
> --------------------------------------------------------------------------
>
>
> --------------------------------------------------------------------------
>
> SOLUTION 3:
> - Based on solution 2, but add a static marker to the generated
> component denoting
> it's suitable to be optimized.
> - Update ApplicationImpl.createComponent() to check for this
> marker and if present,
> add a flag (probably to the attributes map) that can be picked
> up by the rendering
> logic to select the appropriate rendering method. This would
> solve the issue
> with solution 2.
>
> PROBLEMS WITH SOLUTION 3:
> - Adding some overhead to the ApplicationImpl.createComponent()
> code path (probably minor)
> - An application developer completely replaces the Application
> object or overrides createComponent()
> so that nothing gets flagged, so rendering occurs as it does
> now (back to square one).
> --------------------------------------------------------------------------
>
>
> Issues aside, I've implemented solution 2 locally, and have seen a
> very nice improvment in
> the rendering. Specifically, for a similar number of invocations, the
> time rendering pass through
> attributes was reduced to 384 ms.
>
> So, I'm looking for feedback:
> - thoughts on each solution
> - solutions that haven't been considered
> - other problems that haven't been considered
> - Is the performance gain mentioned worth the additional complexity?
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe_at_javaserverfaces.dev.java.net
> For additional commands, e-mail: dev-help_at_javaserverfaces.dev.java.net
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_javaserverfaces.dev.java.net
For additional commands, e-mail: dev-help_at_javaserverfaces.dev.java.net