Hi Leonardo, Thank you again for your response!
On Mon, Jun 28, 2010 at 11:15 PM, Leonardo Uribe <[email protected]> wrote: > Hi Marius > > 2010/6/28 Marius Petoi <[email protected]> > >> Hello, >> >> After looking more to the attributes of the components, here's what I've >> found regarding state saving: >> >> ErrorPageWriter.VISITED_FACET_COUNT - used in the debug page for >> displaying facet components. It is recalculated at each request, so it is >> not necessary in the state. My suggestion would be to introduce a map in >> ErrorPageWriter with number of visited facets for every component >> >> > Yes, sounds good. > Then I shall do it like this. > > >> CompositeComponentResourceTagHandler.ATTACHED_OBJECT_HADLERS_KEY - saved >> in partial state as an attribute. The object is an ArrayList, but my >> suggestion would be to use instead _DeltaList >> >> > This list is not saved on the state, instead, it is removed on > CompositeComponentResourceTagHandler.applyNextHandler, because the purpose > is use it on vdl.retargetAttachedObjects method to apply each of the > handlers related to the current composite component. The problem here is > since this call occur in build time, we can't call getClientId() (it will > cause problems because the parent for most components is null in this > stage), so use a external map here is not possible. The solution is have a > transient property map for each component, but we can't do that without a > change in the spec. > OK. So this is not a problem, if you say that it is not saved in the state. > > > >> ValidatorTagHandlerDelegate.VALIDATOR_ID_EXCLUSION_LIST_KEY - saved in >> partial state as an attribute. This object is also an ArrayList, that I >> think should be a _DeltaList instead. >> >> > Yes, it is possible. > > >> InsertChildrenTag.INSERT_CHILDREN_USED, InsertFacetTag.INSERT_FACET_USED - >> saved in bean if the build is for composite component metadata. Otherwise, >> saved as a component parameter. When used, these parameters are taken only >> from the bean. So practically, the value saved in the attribute map is never >> used...and it is included in the saved state >> >> > Good catch!. It is a mistake. At start > InsertChildrenTag.INSERT_CHILDREN_USED and InsertFacetTag.INSERT_FACET_USED > were used to save that information on component state, but that strategy was > moved to beanDescriptor to allow apply some facelet tags inside a composite > component first and then the ones inside cc:implementation. I'll remove > those lines. > OK. > > >> Also the list parameters saved in the BeanDescriptor objects in the >> CompositeComponentBeanInfo are ArrayList-s. But as they are included in the >> saved state, my suggestion would be to make them _DeltaList-s. >> >> > Note that CompositeComponentBeanInfo implements Externalizable. That means > this object is serialized, so first that object should implement > PartialStateHolder. > > In theory the algorithm we have, try to cache this object and use it across > composite components, but this object is serialized/deserialized on each > save/restore cycle. I think it has sense this object implements > PartialStateHolder interface. > > Thinking about this stuff, I remember one small enhancement I did on > org.apache.myfaces.view.facelets.tag.jsf.ActionSourceRule. The relevant code > is this: > > public void applyMetadata(FaceletContext ctx, Object instance) > { > final MethodExpression methodExpressionOneArg = > _attr.getMethodExpression(ctx, null, ActionSourceRule.ACTION_LISTENER_SIG); > final MethodExpression methodExpressionZeroArg = > _attr.getMethodExpression(ctx, null, ActionSourceRule.ACTION_SIG); > if > (FaceletCompositionContext.getCurrentInstance(ctx).isUsingPSSOnThisView()) > { > ((ActionSource2) instance).addActionListener( > new > PartialMethodExpressionActionListener(methodExpressionOneArg, > methodExpressionZeroArg)); > } > else > { > ((ActionSource2) instance).addActionListener( > new > MethodExpressionActionListener(methodExpressionOneArg, > methodExpressionZeroArg)); > } > } > > In this case MethodExpressionActionListener implements StateHolder, but > PartialMethodExpressionActionListener extends MethodExpressionActionListener > and implements PartialStateHolder. In theory if PSS is used, > PartialMethodExpressionActionListener is better because it reduce the state > size. > > In some parts, facelets uses objects like the one before that are > serialized instead implements StateHolder or PartialStateHolder. It could be > good to know which strategy is better (serialization or implements > PartialStateHolder) and check in which cases it is worth to do it. One > example is CompositeComponentBeanInfo, but it could be others as well. > So what you are saying is that in this case it is better to use implements PartialStateHolder than serialization? I'll look for more situations where serialization is used and we shall discuss whether it is better to use PartialStateHolder instead. Also, could please look over the patches when you have the time and tell me whether there is anything I should do differently. Thank you! Regards, Marius > > regards, > > Leonardo Uribe > > Please tell me if my suggestions are correct. Thank you! >> >> Regards, >> Marius >> >> >> On Mon, Jun 28, 2010 at 11:27 AM, Marius Petoi >> <[email protected]>wrote: >> >>> Hello, >>> >>> I created a ticket and attached a patch for removing the MARK_DELETED >>> attribute from the component. The JIRA ticket is: >>> https://issues.apache.org/jira/browse/MYFACES-2774. Please have a look >>> over it and tell me whether I should modify anything. >>> >>> Regards, >>> Marius >>> >>> >>> On Fri, Jun 25, 2010 at 2:12 PM, Marius Petoi >>> <[email protected]>wrote: >>> >>>> Hello, >>>> >>>> Thank you for the explanations! So, as I see here, it is ok to remove >>>> the "org.apache.myfaces.view.facelets.MARK_DELETED" attribute from the >>>> partial state and include it in the FaceletContext (as a list of components >>>> that are marked for deletion). >>>> >>>> Regards, >>>> Marius >>>> >>>> >>>> On Fri, Jun 25, 2010 at 2:20 AM, Leonardo Uribe <[email protected]>wrote: >>>> >>>>> Hi Martin >>>>> >>>>> 2010/6/24 Martin Marinschek <[email protected]> >>>>> >>>>> Hi Leonardo, >>>>>> >>>>>> > The param "org.apache.myfaces.view.facelets.APPLIED" is used with >>>>>> the web >>>>>> > config param javax.faces.FACELETS_REFRESH_PERIOD, to detect changes >>>>>> on the >>>>>> > template files. If you set this param to 0, facelets stops to add it >>>>>> and >>>>>> > your state size is reduced, so that configuration must be used in >>>>>> production >>>>>> > environments. >>>>>> >>>>>> so that is in all components? and it is only for reloading? wouldn't >>>>>> it be enough to have this once per view? In the view-root attributes >>>>>> map? then, if any of the files which are loaded are younger than this >>>>>> param, we drop the whole thing and reload? I thought the MARK_APPLIED >>>>>> was for something else, but I don't remember too well... I am >>>>>> concerned even about polluting the state while development - it makes >>>>>> the debugging harder. >>>>>> >>>>> >>>>> MM> so that is in all components? >>>>> >>>>> It is applied by DefaultFacelet to the component instance that contains >>>>> a reference to a template. >>>>> There are three cases: >>>>> >>>>> - When the first facelet is applied (call to Facelet.apply(FacesContext >>>>> facesContext, UIComponent parent) ) >>>>> - When a composite component template is applied (call to >>>>> DefaultFacelet.applyCompositeComponent(AbstractFaceletContext ctx, >>>>> UIComponent parent, Resource resource) ) >>>>> - When a template is included (call to >>>>> DefaultFacelet.include(AbstractFaceletContext ctx, UIComponent parent, URL >>>>> url) ) >>>>> >>>>> So it appears based on how the page use templates or composite >>>>> components. >>>>> >>>>> MM> and it is only for reloading? >>>>> >>>>> Yes. >>>>> >>>>> MM> wouldn't it be enough to have this once per view? In the view-root >>>>> attributes map? >>>>> >>>>> If we let it just once per view and it is changed a child template, the >>>>> current view will not be updated because it contains the update time of >>>>> the >>>>> base template. It seems possible to use this attribute just once, but to >>>>> do >>>>> that it is necessary to change the whole algorithm. >>>>> >>>>> MM> if any of the files which are loaded are younger than this param, >>>>> we drop the whole thing and reload? >>>>> >>>>> Looking in deep the current algorithm is not using file modification >>>>> times as I was expecting (so it does not look for changes), instead it is >>>>> using as base time the creation time of the Facelet object. So, the >>>>> algorithm just refresh the templates at specified intervals. >>>>> >>>>> It seems we can do something more simple here and do not pollute the >>>>> state. >>>>> >>>>> >>>>>> > I think it is possible to do something about >>>>>> ComponentSupport.MARK_DELETED. >>>>>> > The algorithm used to refresh a view by facelets uses this param to >>>>>> indicate >>>>>> > when a component should be deleted, but I think we can rewrite the >>>>>> algorithm >>>>>> > to do not use the component attribute map to save this information, >>>>>> because >>>>>> > it is only relevant for the current request. >>>>>> >>>>>> For even less, right? It should really be valid only for one building >>>>>> of the view - can we keep it in some facelet-context attribute, keyed >>>>>> by the component instance (so that the lookup is fast)? >>>>>> >>>>>> >>>>> Yes, that's the idea. >>>>> >>>>> best regards, >>>>> >>>>> Leonardo >>>>> >>>>> >>>>>> > Maybe we should change the keys for example from >>>>>> > "org.apache.myfaces.view.facelets.MARK_DELETED" to something smaller >>>>>> like >>>>>> > "oam.facelets.MARK_DELETED" to save some bytes. >>>>>> >>>>>> ah well, it should go completely. Everything else is only half the >>>>>> rent. >>>>>> >>>>>> best regards, >>>>>> >>>>>> Martin >>>>>> >>>>>> > 2010/6/24 Marius Petoi <[email protected]> >>>>>> > >>>>>> >> Hello, >>>>>> >> >>>>>> >> As you said, Martin, the attribute >>>>>> >> "org.apache.myfaces.view.facelets.APPLIED" is included in the >>>>>> partial >>>>>> >> state. >>>>>> >> This, as well as all the other attributes of the components, as the >>>>>> >> attributeMap in the UIComponentBase is a _ComponentAttributesMap. >>>>>> The put >>>>>> >> method of this map calls afterwards the method in the >>>>>> _DeltaStateHelper. >>>>>> >> This method includes everything in the partial state (as long as >>>>>> the >>>>>> >> initial >>>>>> >> state is marked). This means that all the attributes of a component >>>>>> will >>>>>> >> be >>>>>> >> included in the partial state. I suppose that there are other >>>>>> attributes >>>>>> >> as >>>>>> >> facelets.APPLIED, which don't need to be included in the partial >>>>>> state. >>>>>> >> >>>>>> >> So my suggestion at first would be to add flags to the "put" >>>>>> methods of >>>>>> >> the >>>>>> >> _DeltaStateHelper, in which to decide whether the value added needs >>>>>> to be >>>>>> >> in >>>>>> >> the partial state (and therefore added in the _deltas map), or not >>>>>> (in >>>>>> >> which >>>>>> >> case it will be added only in the _fullstate map). Afterwards, we >>>>>> should >>>>>> >> revise all the attributes and decide whether they need to be in the >>>>>> >> partial >>>>>> >> state or not and call the put methods with the apropriate flag. >>>>>> >> >>>>>> >> What do you think? >>>>>> >> >>>>>> >> Regards, >>>>>> >> Marius >>>>>> >> >>>>>> >> >>>>>> >> On Wed, Jun 23, 2010 at 4:54 PM, Martin Marinschek < >>>>>> [email protected] >>>>>> >> > wrote: >>>>>> >> >>>>>> >>> Hi Marius, >>>>>> >>> >>>>>> >>> I think you will easily find out candidates for a better >>>>>> >>> implementation - just take a look at the partial state of any of >>>>>> the >>>>>> >>> components. Something that comes immediately to my mind is >>>>>> >>> facelets.MARK_APPLIED - this is only relevant in request-scope, >>>>>> but is >>>>>> >>> currently in the partial state, both in Mojarra and MyFaces (last >>>>>> time >>>>>> >>> I looked). >>>>>> >>> >>>>>> >>> Stuff like this needs to be fixed. Tell us if you have more >>>>>> information. >>>>>> >>> >>>>>> >>> best regards, >>>>>> >>> >>>>>> >>> Martin >>>>>> >>> >>>>>> >>> On 6/18/10, Leonardo Uribe <[email protected]> wrote: >>>>>> >>> > Hi >>>>>> >>> > >>>>>> >>> > I think the key classes are UIComponentBase , _DeltaStateHelper >>>>>> and >>>>>> >>> > _DeltaList. Take a look at UIComponentBase.saveState() and >>>>>> >>> > UIComponentBase.restoreState(). Those methods have calls to >>>>>> other >>>>>> >>> methods >>>>>> >>> > that are critical to state saving. I remember UIViewRoot has >>>>>> other >>>>>> >>> methods >>>>>> >>> > too. I think check those classes are a good point to start. >>>>>> >>> > >>>>>> >>> > regards, >>>>>> >>> > >>>>>> >>> > Leonardo Uribe >>>>>> >>> > >>>>>> >>> > 2010/6/18 Marius Petoi <[email protected]> >>>>>> >>> > >>>>>> >>> >> Hello, >>>>>> >>> >> >>>>>> >>> >> In order to study the current performance of state saving, I >>>>>> designed >>>>>> >>> >> a >>>>>> >>> >> small page that I included in the examples for MyFaces 2.0. >>>>>> Also, I >>>>>> >>> wrote >>>>>> >>> >> 2 >>>>>> >>> >> phase listeners that determine the length of the state saved in >>>>>> the >>>>>> >>> >> ExternalContext before the render response phase and before the >>>>>> >>> >> restore >>>>>> >>> >> view >>>>>> >>> >> phase. Do you have any suggestions what components should I >>>>>> start >>>>>> >>> >> analyzing >>>>>> >>> >> the state for? >>>>>> >>> >> >>>>>> >>> >> Regards, >>>>>> >>> >> Marius >>>>>> >>> >> >>>>>> >>> > >>>>>> >>> >>>>>> >>> >>>>>> >>> -- >>>>>> >>> >>>>>> >>> http://www.irian.at >>>>>> >>> >>>>>> >>> Your JSF powerhouse - >>>>>> >>> JSF Consulting, Development and >>>>>> >>> Courses in English and German >>>>>> >>> >>>>>> >>> Professional Support for Apache MyFaces >>>>>> >>> >>>>>> >> >>>>>> >> >>>>>> > >>>>>> >>>>>> >>>>>> -- >>>>>> >>>>>> http://www.irian.at >>>>>> >>>>>> Your JSF powerhouse - >>>>>> JSF Consulting, Development and >>>>>> Courses in English and German >>>>>> >>>>>> Professional Support for Apache MyFaces >>>>>> >>>>> >>>>> >>>> >>> >> >
