dev@javaserverfaces.java.net

Re: Components opting into attributesThatAreSet feature

From: Ryan Lubke <Ryan.Lubke_at_Sun.COM>
Date: Tue, 10 Feb 2009 15:15:29 -0800

Mark,

Would you mind opening an issue to track this enhancement? We're bogged
down with 2.0 work at the moment, so if you can spare time to create
a patch that we can apply locally we can get your request in all that
much sooner.



Mark Collette wrote:
> 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/
>>>>>>>>>>>
>>>>>>>>>>>
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe_at_javaserverfaces.dev.java.net
> For additional commands, e-mail: dev-help_at_javaserverfaces.dev.java.net
>