Without any changes to Mojarra, I was able to update our rendering of
stock h: components to use attributesThatAreSet, which saw, in one test
application, a reduction of 8.4% in the time to render, and a reduction
of 7.7% in the time it takes to do a server push lifecycle! Hopefully,
if we can get that change made to the handleAttribute(-) methods, then
we can see gain like this for all components :)
- Mark Collette
Mark Collette wrote:
> I'm not sure that there's an easy and performant way of getting at a
> contex-param value, from inside of
> UIComponent.getAttributesThatAreSet(boolean). I've though about many
> different ways of signalling to the component that we wish to enable
> this feature. And then I noticed that, in getAttributesThatAreSet(-),
> accessing attributesThatAreSet is not restricted to optimised
> packages, just its creation. Which means that if ICEfaces component
> constructors simply set a List themselves, then this method should
> work. The act of setting attributesThatAreSet could be the means of
> signalling to enable this feature. There would be the state savings
> cost of unnecessary Lists in some cases, but no stock components or
> other third party components would bear that cost.
>
> UIComponent.java
>
> List<String> getAttributesThatAreSet(boolean create) {
>
> String name = this.getClass().getName();
> if (name != null && name.startsWith(OPTIMIZED_PACKAGE)) {
> if (create && attributesThatAreSet == null) {
> attributesThatAreSet = new ArrayList<String>(6);
> }
> }
> return attributesThatAreSet;
>
> }
>
> The problem with that approach is that all the components'
> handleAttribute(String, Object) methods, for when property setter
> methods are directly invoked, do not allow accessing
> attributesThatAreSet if it already exists, when the component is not
> in the optimised package. If we slightly tweak the logic here, then
> hopefully that will allow the components to set the List themselves,
> to enable this optimisation.
>
> HtmlOutputText.java (existing)
>
> private void handleAttribute(String name, Object value) {
> List<String> setAttributes = null;
> String cname = this.getClass().getName();
> if (cname != null && cname.startsWith(OPTIMIZED_PACKAGE)) {
> setAttributes = (List<String>)
> this.getAttributes().get("javax.faces.component.UIComponentBase.attributesThatAreSet");
>
> if (setAttributes == null) {
> setAttributes = new ArrayList<String>(6);
>
> this.getAttributes().put("javax.faces.component.UIComponentBase.attributesThatAreSet",
> setAttributes);
> }
> if (value == null) {
> setAttributes.remove(name);
> } else if (!setAttributes.contains(name)) {
> setAttributes.add(name);
> }
> }
> }
>
> HtmlOutputText.java (proposed)
>
> private void handleAttribute(String name, Object value) {
> List<String> setAttributes = (List<String>)
> this.getAttributes().get("javax.faces.component.UIComponentBase.attributesThatAreSet");
>
> if (setAttributes == null) {
> String cname = this.getClass().getName();
> if (cname != null && cname.startsWith(OPTIMIZED_PACKAGE)) {
> setAttributes = new ArrayList<String>(6);
>
> this.getAttributes().put("javax.faces.component.UIComponentBase.attributesThatAreSet",
> setAttributes);
> }
> }
> if (setAttributes != null) {
> if (value == null) {
> setAttributes.remove(name);
> } else if (!setAttributes.contains(name)) {
> setAttributes.add(name);
> }
> }
> }
>
> I think that the only downside to this change is for third party
> components, that don't support the attributesThatAreSet optimisation,
> if property setter methods are called directly, they might
> unnecessarily allocate the UIComponentBase.attributes AttributesMap,
> assuming that no properties were set through the attributes map
> directly. And even then, the state saving code won't save it, so next
> lifecycle it will be gone. On the plus side, for stock components, if
> handleAttribute(-) is called subsequently, you'll be avoiding checking
> the class name redundantly, since it will only be checked the first time.
>
> - Mark
>
>
>
> Ryan Lubke wrote:
>
>> Mark Collette wrote:
>>
>>> Yes, you're right, it shouldn't be static. I guess with the JSF jars
>>> increasingly being moved from the WAR to a common directory of the
>>> app server, the classloading ramifications are that static fields
>>> are not private to a webapp anymore? Or does the classloader still
>>> segregate the JSF classes?
>>
>>
>> I can't speak to other app servers, but as far as JSF class instances
>> in GF are concerned, they are shared.
>>
>>>
>>> - Mark
>>>
>>>
>>> Ryan Lubke wrote:
>>>
>>>> Mark Collette wrote:
>>>>
>>>>> This task has languished on our end because the scope of necessary
>>>>> changes, to every single component's state saving, both in Mojarra
>>>>> and in our suite, is a little too large. Hopefully a simpler
>>>>> approach might be sufficient. I was wondering if it would be
>>>>> possible to just set a context-param, that would globally enable
>>>>> use of attributesThatAreSet. Applications could turn it on if the
>>>>> components they used would benefit from it, and could turn it off
>>>>> if the state savings cost was too great. Regardless of the
>>>>> context-param, the standard components would behave as they
>>>>> already do. And in fact, if the context-param was true, it would
>>>>> run ever so slightly faster, since it wouldn't have to do the
>>>>> string test on the class name.
>>>>
>>>>
>>>>
>>>> One problem I see with the suggestion is:
>>>>
>>>> private static boolean ALLOW_ALL_USE_ATTRIBUTES_THAT_ARE_SET = ...;
>>>>
>>>> If the class is shared across all webapps, then the value assigned
>>>> there
>>>> is shared by all of them. I don't think that behavior is
>>>> desirable. It should
>>>> be per-webapp.
>>>>
>>>>>
>>>>>
>>>>> public abstract class UIComponent implements StateHolder {
>>>>> // Old way
>>>>> List<String> getAttributesThatAreSet(boolean create) {
>>>>> String name = this.getClass().getName();
>>>>> if (name != null && name.startsWith(OPTIMIZED_PACKAGE)) {
>>>>> if (create && attributesThatAreSet == null) {
>>>>> attributesThatAreSet = new ArrayList<String>(6);
>>>>> }
>>>>> }
>>>>> return attributesThatAreSet;
>>>>> }
>>>>>
>>>>>
>>>>> // New way
>>>>> private static boolean ALLOW_ALL_USE_ATTRIBUTES_THAT_ARE_SET =
>>>>> ...;
>>>>>
>>>>> List<String> getAttributesThatAreSet(boolean create) {
>>>>> if (create && attributesThatAreSet == null) {
>>>>> String name;
>>>>> if ( ALLOW_ALL_USE_ATTRIBUTES_THAT_ARE_SET ||
>>>>> ((name = this.getClass().getName()) != null &&
>>>>> name.startsWith(OPTIMIZED_PACKAGE)) {
>>>>> attributesThatAreSet = new ArrayList<String>(6);
>>>>> }
>>>>> }
>>>>> return attributesThatAreSet;
>>>>> }
>>>>> }
>>>>>
>>>>>
>>>>> Mark Collette
>>>>> ICEsoft Technologies, Inc.
>>>>> http://mark-icefaces-reflections.blogspot.com/
>>>>>
>>>>>
>>>>> Ryan Lubke wrote:
>>>>>
>>>>>> Mark Collette wrote:
>>>>>>
>>>>>>> Maybe the list could always track the set attributes, without
>>>>>>> requiring the component be a stock JSF component? It would just
>>>>>>> be the pass-through attribute rendering code that would impose
>>>>>>> the limitation, that it would only use the list for stock JSF
>>>>>>> components?
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> Would like to have a way to say what components so we don't force
>>>>>> a state saving penalty on those component sets that don't depend
>>>>>> on an implementation specific feature.
>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Mark Collette
>>>>>>> ICEsoft Technologies, Inc.
>>>>>>> http://mark-icefaces-reflections.blogspot.com/
>>>>>>>
>>>>>>>
>>>>>>> Ryan Lubke wrote:
>>>>>>>
>>>>>>>> Mark Collette wrote:
>>>>>>>>
>>>>>>>>> Are there any limitations on adding public or protected
>>>>>>>>> methods into UIComponent or UIComponentBase?
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> Yes, it can't be done as the TCK signature tests would fail the
>>>>>>>> build.
>>>>>>>>
>>>>>>>> Any changes to the API (public or protected) must be backed by
>>>>>>>> the specification.
>>>>>>>>
>>>>>>>>>
>>>>>>>>> Mark Collette
>>>>>>>>> ICEsoft Technologies, Inc.
>>>>>>>>> http://mark-icefaces-reflections.blogspot.com/
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Ryan Lubke wrote:
>>>>>>>>>
>>>>>>>>>> Mark Collette wrote:
>>>>>>>>>>
>>>>>>>>>>> Is there any way for third party components to opt into
>>>>>>>>>>> making use of the attributesThatAreSet feature?
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Probably not given the changes that were made for this
>>>>>>>>>> feature in 1.2_10.
>>>>>>>>>>
>>>>>>>>>>> Or any plans to facilitate that, in the future?
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> No plans at this point. Patches are welcome.
>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Mark Collette
>>>>>>>>>>> ICEsoft Technologies, Inc.
>>>>>>>>>>> http://mark-icefaces-reflections.blogspot.com/
>>>>>>>>>>
>>>>>>>>>>