You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@myfaces.apache.org by "Blake Sullivan (JIRA)" <de...@myfaces.apache.org> on 2009/12/31 23:12:29 UTC

[jira] Created: (TRINIDAD-1668) Speed up UIXComponent.getId()

Speed up UIXComponent.getId()
-----------------------------

                 Key: TRINIDAD-1668
                 URL: https://issues.apache.org/jira/browse/TRINIDAD-1668
             Project: MyFaces Trinidad
          Issue Type: Improvement
    Affects Versions:  1.2.12-plugins 
         Environment: All
            Reporter: Blake Sullivan


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
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

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




-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (TRINIDAD-1668) Speed up UIXComponent.getId()

Posted by "Blake Sullivan (JIRA)" <de...@myfaces.apache.org>.
     [ https://issues.apache.org/jira/browse/TRINIDAD-1668?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Blake Sullivan updated TRINIDAD-1668:
-------------------------------------

    Status: Patch Available  (was: Open)

> Speed up UIXComponent.getId()
> -----------------------------
>
>                 Key: TRINIDAD-1668
>                 URL: https://issues.apache.org/jira/browse/TRINIDAD-1668
>             Project: MyFaces Trinidad
>          Issue Type: Improvement
>    Affects Versions:  1.2.12-plugins 
>         Environment: All
>            Reporter: Blake Sullivan
>         Attachments: JIRA_1668_1_2_12_2.patch
>
>   Original Estimate: 24h
>  Remaining Estimate: 24h
>
> 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
> 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
> 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

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (TRINIDAD-1668) Speed up UIXComponent.getId()

Posted by "Blake Sullivan (JIRA)" <de...@myfaces.apache.org>.
     [ https://issues.apache.org/jira/browse/TRINIDAD-1668?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Blake Sullivan updated TRINIDAD-1668:
-------------------------------------

    Resolution: Fixed
        Status: Resolved  (was: Patch Available)

Implement JIRA-1668 by storing the id attribute on the UIXComponentBase itself and modifying the UIXFacesBeanImpl to delegate back to the UIXComponentBase for id access through the AttributeMap.  I also cleaned up the way that ids and clientIds are generated so that getId() will never return a null id. In order to make the UIXComponentBase change backwards compatible, I moved the UIXFacesBeanImpl and UIXEditableFacesBeanImpl classes to api where our customers can take advantage of them and UIXComponentBase only caches the id locally if it is attached to an UIXFacesBeanImpl directly or indirectly through any levels of FacesBeanWrapppers.  In addition, several tests should have been extending FacesTestCase and were not, so those are now fixed.  Also, UIXEditableFacesBeanImpl now catches case where it is attached to a non UIXEditableValue component, which caught the fact that we had this incorrect entry:
### DON'T KNOW WHY THE NEXT LINE SHOULD BE NECESSARY!
org.apache.myfaces.trinidad.component.UIXSelectRange|org.apache.myfaces.trinidad.ChoiceBar=org.apache.myfaces.trinidadinternal.bean.UIXEditableFacesBeanImpl

in the faces-bean.properties

> Speed up UIXComponent.getId()
> -----------------------------
>
>                 Key: TRINIDAD-1668
>                 URL: https://issues.apache.org/jira/browse/TRINIDAD-1668
>             Project: MyFaces Trinidad
>          Issue Type: Improvement
>    Affects Versions:  1.2.12-plugins 
>         Environment: All
>            Reporter: Blake Sullivan
>         Attachments: JIRA_1668_1_2_12_2.patch
>
>   Original Estimate: 24h
>  Remaining Estimate: 24h
>
> 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
> 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
> 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

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (TRINIDAD-1668) Speed up UIXComponent.getId()

Posted by "Blake Sullivan (JIRA)" <de...@myfaces.apache.org>.
    [ https://issues.apache.org/jira/browse/TRINIDAD-1668?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12829388#action_12829388 ] 

Blake Sullivan commented on TRINIDAD-1668:
------------------------------------------

There is a difference in behavior with the code--if the set of available keys is checked, the ID won't be in the set.  I'll check in a fix

> Speed up UIXComponent.getId()
> -----------------------------
>
>                 Key: TRINIDAD-1668
>                 URL: https://issues.apache.org/jira/browse/TRINIDAD-1668
>             Project: MyFaces Trinidad
>          Issue Type: Improvement
>    Affects Versions:  1.2.12-core
>         Environment: All
>            Reporter: Blake Sullivan
>            Assignee: Blake Sullivan
>             Fix For: 1.2.13-core 
>
>         Attachments: JIRA_1668_1_2_12_2.patch
>
>   Original Estimate: 24h
>  Remaining Estimate: 24h
>
> 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
> 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
> 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

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.