myfaces-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Blake Sullivan <blake.sulli...@oracle.com>
Subject Re: [TRINIDAD][API]TRINIDAD-1669 Improve transient memory consumption of UIXComponentBase.getClientId()
Date Tue, 05 Jan 2010 00:37:05 GMT
Andrew Robinson said the following On 1/4/2010 2:38 PM PT:

Andrew,

Thanks for commenting on this.
> Why isn't UIXComponentBase caching the client ID?
Because of stamping and because I suspect that FacesBean would have 
complicated things.
>  The JSF core
> components already cache their client ID. Part of the spec. is that
> setId() calls on the component clear the cache.
Can you point me to the section, where it says this?  This behavior is 
not part of section 3.1.6 of the JSF 2.0 Specification (which is wrong 
anyway, as the behavior it describes would preclude the stamping 
implemented by UIData.  And is it just me or do others think that it is 
backwards that the specification would actually tell the reader to look 
in the javadoc to find out what the specified behavior is?)

>  In Trinidad, we must
> recursively call setId on child components when indexes change
> (tr:iterator, tr:table, etc.) so that JSF components function
> correctly.
I believe this is an JSF RI implementation detail.  I see nothing in the 
specification that says that calling setId() will clear out the current 
clientId even if the passed in id is identical to the component's 
current id.  This behavior is also not exactly performant as each row 
change for any reason requires a walk of the row subtree to clear out 
the clientIds.
>  Since we already have to do the work of clearing the cached
> client IDs for JSF components, can we not take advantage of this for
> Trinidad components? I would think that this would be a large
> performance improvement.
>   
We if we put in JIRA-1668--Speed up UIXComponent.getId(), we could, 
since that JIRA solves the problem of how to add programmatic logic to 
get/setId() as a side-effect.  The issues then would be quality and some 
performance.  The quality part would be double-checking that the 
StampState class (which implements the setId resetting behavior) is 
being used correctly in all cases (since we would only notice right now 
if non UIComponents were used) and checking whether we will have 
problems with tag implementations that bogusly call getClientId().

The two mechanisms are complimentary, but I would prefer not to make the 
api change if the caching change is good enough.

-- Blake Sullivan
> Do you feel that your proposed changes would give us better
> performance, or perhaps could we combine the two?
>
> -Andrew
>
> On Thu, Dec 31, 2009 at 4:23 PM, Blake Sullivan
> <blake.sullivan@oracle.com> wrote:
>   
>> Calling UIXComponentBase.getClientId consumes a great deal of transient
>> memory.  Under light loads, this doesn't matter--the objects are extremely
>> short-lived and are allocated out of the first-generation heap.  However,
>> when large numbers of users are accessing the server simultaneously these
>> allocations contribute to first-generation heap exhaustion and
>> first-generation heap GC's when deeply nested NamingContainers are used.
>>
>> There are two reasons that large amounts of transient memory is consumed in
>> these cases:
>> 1) UIXComponentBase doesn't cache clientIds because the clientIds are partly
>> determined by the component's ancestors and there are cases (such as
>> stamping), where multiple clientIds may map to a single component instance
>>
>> 2) clientIds are generated recursively by:
>> a) calling getContainerClientId() and appending the NamingContainer
>> separator and the component's id to the result
>> b) getContainerClientId() is implemented by calling getContainerClientId()
>> and doing likewise
>>
>> So, each NamingContainer in the hierarchy is going to:
>> 1) Get it's ancestor's container clientId and if one exists
>> 2) Get it's id attribute
>> 3) Allocate a StringBuilder to contain these two Strings, append them
>> together
>> 4) Convert the StringBuilder to a String and return the result
>>
>> An earlier JIRA used a ThreadLocal StringBuilder to remove the StringBuilder
>> allocation in step 3) in the common case, halving the transient memory
>> usage, however we still have the String allocations made necessary by the
>> use of   String getContainerClientId(FacesContext context, UIComponent
>> child).
>>
>> For a 20 row table containing 10 columns nested four NamingContainers deep
>> (counting the table as one of these), we end up with 1000 String
>> allocations, which wouldn't necessarily be that bad if the size of the
>> Strings wasn't increasing and if the Rendering code was the only code
>> calling getClientId() (InvokeOnComponent is the primary culprit here, though
>> replacing invokeOnComponent calls with visitTree calls improves things).
>>
>> The proposed solution is to replace generating new Strings at each
>> NamingContainer level with appending the NamingContainer ids into a
>> StringBuilder (in fact, the shared StringBuilder) passed to the appending
>> code--a String is only generated when the returning the value from
>> getClientId().  In scalability testing, this change has been worth about 8%.
>>
>> The advantages of this approach are:
>> 1) If the component code compiles, the code will almost certainly work
>> correctly
>> 2) It clientId caching is also used, this approach speeds up generation of
>> the cached result
>>
>> The disadvatanges of this approach is:
>> 1) Any overrides of getClientId() or getContainerClientId() must be changed
>> to overrides of appendClientId() or appendContainerClientId().  To enforce
>> this, getClientId() and getContainerClientId() are made final on
>> UIXComponentBase.  This, is of course, an incompatible api change
>>
>> The new/changed apis on UIXComponentBase:
>>
>>  /**
>>  * Appends the container's clientId for the requesting child to the
>> StringBuilder, returning the passed in StringBuilder.
>>  * Component implementations are only allowed to mutate the StringBuilder
>> other than to append.
>>  * Subclasses that wish to modify the clientIds returned for their children
>> should override this method rather than
>>  * <code>getContainerClientId</code>.
>>  * @param context FacesContext
>>  * @param child Optional child component that is requesting the container's
>>  *              clientId
>>  * @param clientIdAppendable StringBuilder to append the container's
>> clientId to
>>  * @see #getContainerClientId(FacesContext, UIComponent)
>>  */
>>  public StringBuilder appendContainerClientId(
>>   FacesContext context,
>>   UIComponent child,
>>   StringBuilder clientIdAppendable)
>>
>>  /**
>>  * Appends the clientId of this component to the StringBuilder, returning
>> the passed in StringBuilder.
>>  * Component implementations typically only mutate the StringBuilder to
>> append.
>>  * Subclasses that wish to modify the clientIds that they return should
>> override this method rather than
>>  * <code>getClientId</code>.
>>  * @param context FacesContext
>>  * @param clientIdAppendable StringBuilder to append the component's
>> clientId to
>>  * @return the clientIdAppendable StringBuilder passed in as the
>> clientIdAppendable parameter
>>  * @see #getClientId
>>  */
>>  public StringBuilder appendClientId(FacesContext context, StringBuilder
>> clientIdAppendable)
>>
>>  /**
>>  * Final override of getContainerClientId to make
>> <code>appendContainerClientId</code>
>>  * the supported hook for modifying the clientIds of a component's children.
>>  * @see #appendContainerClientId
>>  */
>>  @Override
>>  public String getContainerClientId(FacesContext context)
>>
>>  /**
>>  * Final override of getContainerClientId to make
>> <code>appendContainerClientId</code>
>>  * the supported hook for modifying the clientIds of a component's children.
>>  * The implementation uses <code>appendContainerClientId</code> to calculate
>> the
>>  * the container's clientId prefix with far fewer temporary Strings than
>>  * the class JSF implementation.
>>  * @param context FacesContext
>>  * @param child Optional child component that is requesting the container's
>>  *              clientId
>>  * @return the clientId prefix to add to the child's id
>>  * @see #appendContainerClientId
>>  */
>>  @Override
>>  public final String getContainerClientId(FacesContext context, UIComponent
>> child)
>>
>>  /**
>>  * Final override of getClientId to make <code>appendClientId</code>
>>  * the supported hook for modifying the clientIds of a component.
>>  * The implementation uses <code>appendClientId</code> to calculate the
>>  * the component's clientId with far fewer temporary Strings than
>>  * the class JSF implementation.
>>  * @param context FacesContext
>>  * @return the clientId
>>  * @see #appendClientId
>>  */
>>  @Override
>>  public final String getClientId(FacesContext context)
>>
>>     


Mime
View raw message