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