Blake,
For 1, both possibilities exist. However, I would prefer them to not be
available on the FacesBean from a performance PoV. Those don't have indexed
property keys anyway so the lookup for them is actually quite inefficient.
That would requires some additional changes to the state saving though.
So, we would have:
For predefined properties:
1. Direct access:
UIComponent.getX() --> FacesBean.getProperty(X_KEY) ==> O(1), one indexed
map loopkup
2. FacesBean access (in renderers):
FacesBean.getProperty(X_KEY) ==> O(1), one indexed map loopkup
3. Attribute map access:
UIComponent.getAttributes().get("x") --> UIComponent.getX() -->
FacesBean.getProperty(X_KEY) ==> O(1), one hashed map lookup
(String.hashCode is cached), one reflection call, one indexed map loopkup
For custom properties:
3. Attribute map access:
UIComponent.getAttributes().get("x") ==> O(1), one hashed map lookup
If we keep setting the custom properties in FacesBean, then the AttributeMap
must also have a link to the FacesBean and the algorithm would be
Accessor accessor = getAccessor(propertyName);
if (accessor == null)
{
// custom property, use the faces bean directly
PropertyKey propertyKey = _bean.getType().findKey(propertyName);
if (propertyKey == null)
propertyKey = PropertyKey.createPropertyKey(propertyName);
return _bean.getProperty(propertyKey);
}
else
{
// predefined property
return accessor.get(component);
}
private Accessor getAccessor(String propertyName)
{
// Using an accessor cache should be required, sadly Method is not
Serializable,
// but it would still be possible to cache it in a semi static
ClassLoaderLocal Map<Class<?>, Map<String, Accessor>> ...
}
private static class Accessor
{
private Method getter;
private Method setter;
private Class<?> type;
public Object get(Object o)
{
return getter.invoke(o);
}
// ...
}
Regards,
~ Simon
On Tue, Jan 5, 2010 at 4:31 PM, Blake Sullivan <blake.sulli...@oracle.com>wrote:
> Simon,
>
> For part 1), are you proposing that we stop overriding getAttributes()? If
> so, private implementation properties used by the component and set by using
> setAttribtue(), would not be available on the FacesBean. So I assume that
> you are suggesting that we change the components to set these on the
> FacesBean directly in these cases.
>
> I did a quick grep and the components and they are using the attributeMap.
> It is unclear how many of these would be left if we removed all of the cases
> where the direct accessor could be used, and the cases where we would switch
> to direct FacesBean access, however these case do suffer from the worst of
> all worlds as far as performance, since the pay the cost of both reflection
> and Map access.
>
> -- Blake Sullivan
>
>
> Simon Lessard said the following On 1/5/2010 12:32 PM PT:
>
> Hi Blake,
>
> Actually it's harsher/simpler than that. Assuming that .getAttributes() is
> very rarely used in a Trinidad application (exception for custom
> attributes).
>
> 1. Have AttributeMap work exactly like standard JSF's AttributeMap. That is,
> always call the getter/setter on the component (which in turn will use the
> FacesBean if needed). For custom properties, they could either be stored in
> the FacesBean or on the component itself
> 2. Handle the id attribute manually for state saving purposes in
> UIXComponentBase
>
> Point 1. does impact performances vs. direct FacesBean access when accessing
> defined properties, but all Trinidad applications most likely directly use
> FacesBean.getProperty(PropertyKey) directly, like all our renderer do. For
> custom properties, there's absolutely no hit.
>
>
> Regards,
>
> ~ Simon
>
> On Tue, Jan 5, 2010 at 3:24 PM, Blake Sullivan <blake.sulli...@oracle.com>
> <blake.sulli...@oracle.com>wrote:
>
>
>
> Is your suggestion that we
> 1) Add a new Map(String, Object>) implementation that takes both the
> FacesBean and the UIComponent
> 2) Explicitly test for the id attribute in all of the accessor and mutator
> functions, in addition to the the Sets returned
> 3) Override the state saving/restoration code to explicitly handle id
>
> -- Blake Sullivan
>
> Simon Lessard said the following On 1/5/2010 12:08 PM PT:
>
> Have the AttributeMap call the getId/setId. The contract for the Map
> returned by getAttributes is supposed to call the getter/setter method on
> the component anyway,
> fromhttp://java.sun.com/javaee/5/docs/api/javax/faces/component/UIComponent.html#getAttributes%28%29
> :
>
>
>
>
> - get() - If the property is readable, call the getter method and
> return the returned value (wrapping primitive values in their corresponding
> wrapper classes); otherwise throw IllegalArgumentException.
> - put() - If the property is writeable, call the setter method to set
> the corresponding value (unwrapping primitive values in their corresponding
> wrapper classes). If the property is not writeable, or an attempt is made
> to
> set a property of primitive type to null, throw
> IllegalArgumentException.
>
>
>
>
> Regards,
>
> ~ Simon
>
>
> On Tue, Jan 5, 2010 at 2:50 PM, Blake Sullivan <blake.sulli...@oracle.com>
> <blake.sulli...@oracle.com> <blake.sulli...@oracle.com>
> <blake.sulli...@oracle.com>wrote:
>
>
>
> The reason is that we need to support AttributeMap/component accessor
> equivalence--get/set of the id attribute through the Map is supposed to work
> correctly. The ValueExpression only exists to make this work.
>
> -- Blake Sullivan
>
> Simon Lessard said the following On 1/5/2010 10:57 AM PT:
>
> Hi,
>
> Why not simply NOT support a PropertyKey for the id attribute? I know it
> isn't consistent with the other properties, but id is a very special case
> not supporting EL anyway. In all the project I ever did, I never used
> FacesBean.getProperty(ID_
> PROPERTY_KEY). The only drawback I would see is if
> the component's id actually need to be read in a property getter method in a
> renderer which receive only the FacesBean instance and not the component
> itself. That would be much faster than a custom ValueExpression and the
> memory footprint would also be better.
>
> Regards,
>
> ~ Simon
>
> On Tue, Jan 5, 2010 at 1:49 PM, Matthias Wessendorf <mat...@apache.org>
> <mat...@apache.org> <mat...@apache.org> <mat...@apache.org>
> <mat...@apache.org> <mat...@apache.org> <mat...@apache.org>
> <mat...@apache.org>wrote:
>
>
>
> On Thu, Dec 31, 2009 at 11:12 PM, Blake Sullivan<blake.sulli...@oracle.com>
> <blake.sulli...@oracle.com> <blake.sulli...@oracle.com>
> <blake.sulli...@oracle.com> <blake.sulli...@oracle.com>
> <blake.sulli...@oracle.com> <blake.sulli...@oracle.com>
> <blake.sulli...@oracle.com> wrote:
>
>
> UIComponent.getId() is by far the most requested component attribute.
>
>
> There
>
>
> are a number of reasons for this:
> 1) The JSF RI has an issue in the JSP-JSF integration which causes
>
>
> getId()
>
>
> to be called n^2 times where n is the number of children a component has
>
>
> I guess this is true for MyFaces as well, right?
>
>
>
> 2) getClientId() calls getId()
> 3) FindComponent calls getId()
> 4) The tree visiting code trades off calls to getClientId() for calls to
> getId()
>
> FacesBean optimizes attribute Map access at the expense of access
>
>
> directly
>
>
> through the component. The the extent that Renderers are Components are
> accessing the attributes through the attribute Map, this is fine, however
> even the Renderers access attributes common to all UIComponents such as
>
>
> id()
>
>
> through the component directly. Considering the huge number of times
>
>
> that
>
>
> the the id is accessed (for some renders, this was 8% of the rendering
> time), it makes sense to optimize this path.
>
> The proposal is to:
> 1) Store the id an an instance variable on the UIXComponent
> 2) Add a new capability flag to PropertyKey indicating that the property
>
>
> is
>
>
> actually stored elsewhere using a ValueExpression will be stored as the
> property's value in the PropertyMap. For access through the FacesBean,
>
>
> the
>
>
> ValueExpression will be evaluated to get/set the actual value
> 3) For state saving the ValueExpression is used to retrieve the actual
>
>
> value
>
>
> and for state restoration the ValueExpression (which has been
>
>
> rebootstrapped
>
>
> by the UIXComponent) is used to write the value back
> 4) Instead of setting the id attribute in the FacesBean, UIXComponent
>
>
> stores
>
>
> it locally and sets an ValueExpression implementation into the FacesBean
> that retrieves the value from the UIXComponent
>
>
> +1 on api/patch
>
>
>
> API Changes:
>
> PropertyKey:
>
> add
>
> /**
> * Capability indicating that values for this property should be
> * be stored and retrieved through a ValueExpression rather than on the
> * FacesBean itself
> */
> static public final int CAP_VALUE_EXPRESSION_IMPLEMENTATION = 16;
>
> /**
> * Returns <code>true</code> if property values for this key are set and
>
>
> get
>
>
> * using a ValueExpression rather than storing the value in the
>
>
> FacesBean.
>
>
> * @return <code>true</code> if properties values for this key are
>
>
> retrieved
>
>
> * with a ValueExpression.
> */
> public boolean usesValueExpressionAsImplementation()
>
> After this change id retrieval doesn't make the 1% YourKit profiler hot
>
>
> spot
>
>
> cut off
>
>
>
>
>
> --
> Matthias Wessendorf
>
> blog: http://matthiaswessendorf.wordpress.com/
>
>
> sessions: http://www.slideshare.net/mwessendorf
> twitter: http://twitter.com/mwessendorf
>
>
>
>
>
>
>
