myfaces-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Marius Petoi <marius.pe...@codebeat.ro>
Subject Re: [GSOC] State saving performance improvements in MyFaces 2.0
Date Tue, 29 Jun 2010 07:29:28 GMT
Hi Leonardo,

Thank you again for your response!

On Mon, Jun 28, 2010 at 11:15 PM, Leonardo Uribe <lu4242@gmail.com> wrote:

> Hi Marius
>
> 2010/6/28 Marius Petoi <marius.petoi@codebeat.ro>
>
>> 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 <marius.petoi@codebeat.ro>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 <marius.petoi@codebeat.ro>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 <lu4242@gmail.com>wrote:
>>>>
>>>>> Hi Martin
>>>>>
>>>>> 2010/6/24 Martin Marinschek <mmarinschek@apache.org>
>>>>>
>>>>> 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 <marius.petoi@codebeat.ro>
>>>>>> >
>>>>>> >> 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 <
>>>>>> mmarinschek@apache.org
>>>>>> >> > 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 <lu4242@gmail.com>
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 <marius.petoi@codebeat.ro>
>>>>>> >>> >
>>>>>> >>> >> 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
>>>>>>
>>>>>
>>>>>
>>>>
>>>
>>
>

Mime
View raw message