Re: [core] performance: avoid wrapped EL Expressions
Hi
Checking this patch I founded one use case where the hack proposed could fail:
I found it in this link:
http://seamframework.org/Documentation/CreatingACompositeAjaxifiedForm
In this case, things could be rewritten to this:
I think in this case the second syntax is better, so we can just
ignore it. After all, if anyone wants to preserve the previous
behavior he/she can always disable the optimization. Note the proposal
is this optimization should be enabled by default.
Checking this stuff I founded c:set behavior is not very clear. This
is what it says on the spec:
"... Sets the result of an expression evaluation based on the value of
the attributes. If "scope" the is present, but has a zero length or is
equal to the string "page", TagException is thrown with an informative
error message, ensuring page location information is saved. ..."
This tag comes as a replacement for jsp c:set tag. Given the previous
description you can assume c:set setting only "var" and "value"
variables do something over a "page" scope, which in this case are
related to VariableMapper wrappers. But in this example:
cset2.xhtml
cset2_1.xhtml
#{pages} expression should not be resolved, but the current code set
pages var to 20 and you can see the value outside. Not good.
The same is true for this code:
cset3.xhtml
cset3_1.xhtml
Not good too. I think we should "restrict" c:set created vars using
"page" default scope to the current page being processed, and do not
allow this buggy behavior. Really there are 5 cases where
VariableMapper is used:
1. ui:composition with ui:param
2. ui:decorate with ui:param
3. c:forEach
4. c:set without using scope
5. user tag handler
Number 3 is not a problem, because is only inside c:forEach tag.
Number 1. and 2. and maybe 3. are a problem, because ui:param
declarations should pass from template client to templates (I'm not
have clear this but I'm working on it). Number 4 works different to 1,
2, so expressions defined there should be handled differently, even if
both solutions "share" the same VariableMappers.
This deserves and requires more investigation, but it is not an
stopper for the patch proposed.
If no objections I'll commit the patch proposed soon.
regards,
Leonardo Uribe
2011/6/5 Kito Mann :
> Ah, okay. Thanks for the explanation, Leonardo.
> ---
> Kito D. Mann | twitter: kito99 | Author, JSF in Action
> Virtua, Inc. | http://www.virtua.com | JSF/Java EE training and consulting
> http://www.JSFCentral.com - JavaServer Faces FAQ, news, and info | twitter:
> jsfcentral
> +1 203-404-4848 x3
>
> * Listen to the latest headlines in the JSF and Java EE
> newscast: http://blogs.jsfcentral.com/roller/editorsdesk/category/JSF+and+Java+EE+Newscast
> * See you at JAX and JSF Summit 2011 June 20-23rd in San
> Jose: http://jaxconf.com/
>
>
> On Fri, Jun 3, 2011 at 6:45 PM, Leonardo Uribe wrote:
>>
>> Hi Kito
>>
>> No, this will reduce the number of value/method expressions created
>> each request, but the calls to getters during request will be the
>> same.
>>
>> In my understanding, some additional getter calls are triggered by
>> effect of ve.getType(). The only way to prevent that is define the
>> type beforehand (for example, tomahawk has valueType property in some
>> extended standard components).
>>
>> Anyway, the optimization proposed is significant, because create such
>> expressions takes valuable cpu and memory resources.
>>
>> Leonardo
>>
>> 2011/6/3 Kito Mann :
>> > Leonardo, would this reduce the number of calls to getters during
>> > request
>> > processing?
>> > ---
>> > Kito D. Mann | twitter: kito99 | Author, JSF in Action
>> > Virtua, Inc. | http://www.virtua.com | JSF/Java EE training and
>> > consulting
>> > http://www.JSFCentral.com - JavaServer Faces FAQ, news, and info |
>> > twitter:
>> > jsfcentral
>> > +1 203-404-4848 x3
>> >
>> > * Listen to the latest headlines in the JSF and Java EE
>> >
>> > newscast: http://blogs.jsfcentral.com/roller/editorsdesk/category/JSF+and+Java+EE+Newscast
>> > * See you at JAX and JSF Summit 2011 June 20-23rd in San
>> > Jose: http://jaxconf.com/
>> >
>> >
>> > On Thu, Jun 2, 2011 at 10:10 PM, Leonardo Uribe
>> > wrote:
>> >>
>> >> Hi
>> >>
>> >> I have attached another patch to MYFACES-3160. This one has the
>> >> ability to detect if the expression uses some variable resolved by
>> >> facelets VariableMapper wrappers and if so, indicate the expression
>> >> cannot be cached.
>> >>
>> >> This solution will work better, because it only creates Value/Method
>> >> expressions when it is necessary. This is a great optimization,
>> >> because in practice most expressions are bound to managed beans, so in
>> >> practice it will make pages render faster (because EL parsing is only
>> >> done once and a lot of objects will not be created) and the memory
>> >> usage will be lower (less instances means gc works less).
>> >>
>> >> The only par
Re: [core] performance: avoid wrapped EL Expressions
Ah, okay. Thanks for the explanation, Leonardo.
---
Kito D. Mann | twitter: kito99 | Author, JSF in Action
Virtua, Inc. | http://www.virtua.com | JSF/Java EE training and consulting
http://www.JSFCentral.com - JavaServer Faces FAQ, news, and info | twitter:
jsfcentral
+1 203-404-4848 x3
* Listen to the latest headlines in the JSF and Java EE newscast:
http://blogs.jsfcentral.com/roller/editorsdesk/category/JSF+and+Java+EE+Newscast
* See you at JAX and JSF Summit 2011 June 20-23rd in San Jose:
http://jaxconf.com/
On Fri, Jun 3, 2011 at 6:45 PM, Leonardo Uribe wrote:
> Hi Kito
>
> No, this will reduce the number of value/method expressions created
> each request, but the calls to getters during request will be the
> same.
>
> In my understanding, some additional getter calls are triggered by
> effect of ve.getType(). The only way to prevent that is define the
> type beforehand (for example, tomahawk has valueType property in some
> extended standard components).
>
> Anyway, the optimization proposed is significant, because create such
> expressions takes valuable cpu and memory resources.
>
> Leonardo
>
> 2011/6/3 Kito Mann :
> > Leonardo, would this reduce the number of calls to getters during request
> > processing?
> > ---
> > Kito D. Mann | twitter: kito99 | Author, JSF in Action
> > Virtua, Inc. | http://www.virtua.com | JSF/Java EE training and
> consulting
> > http://www.JSFCentral.com - JavaServer Faces FAQ, news, and info |
> twitter:
> > jsfcentral
> > +1 203-404-4848 x3
> >
> > * Listen to the latest headlines in the JSF and Java EE
> > newscast:
> http://blogs.jsfcentral.com/roller/editorsdesk/category/JSF+and+Java+EE+Newscast
> > * See you at JAX and JSF Summit 2011 June 20-23rd in San
> > Jose: http://jaxconf.com/
> >
> >
> > On Thu, Jun 2, 2011 at 10:10 PM, Leonardo Uribe
> wrote:
> >>
> >> Hi
> >>
> >> I have attached another patch to MYFACES-3160. This one has the
> >> ability to detect if the expression uses some variable resolved by
> >> facelets VariableMapper wrappers and if so, indicate the expression
> >> cannot be cached.
> >>
> >> This solution will work better, because it only creates Value/Method
> >> expressions when it is necessary. This is a great optimization,
> >> because in practice most expressions are bound to managed beans, so in
> >> practice it will make pages render faster (because EL parsing is only
> >> done once and a lot of objects will not be created) and the memory
> >> usage will be lower (less instances means gc works less).
> >>
> >> The only part this does not have any effect is when there is a
> >> ValueExpression directly on the markup. In this case, I think since
> >> org.apache.myfaces.view.facelets.compiler.UIInstructionHandler is
> >> final (that means it is inmutable), put a volatile variable for cache
> >> such cases will make it a mutable class, so it cannot take advantage
> >> of some compiler optimizations. Maybe we can create two classes, one
> >> to handle markup without EL and other with EL. Anyway, the most
> >> critical point is expressions on attributes (changes on facelets
> >> compiler has to be done very carefully).
> >>
> >> JK> I would really like to see some prototyping work for JSF 2.2 in this
> >> JK> area directly in a MyFaces core branch!
> >>
> >> The patch proposed on MYFACES-3160 is for MyFaces 2.0 and 2.1. After
> >> the latest patch I think we don't really need some work on a EL
> >> implementation (see explanations below).
> >>
> >> MK>>
> >> MK>> we need to avoid unnecessary ValueExpression instances.
> >>
> >> Yes, sure!. I know this optimization will be very useful, and it will
> >> do better use of available resource (memory and cpu).
> >>
> >> MK>>
> >> MK>> Here is one idea: because we cannot wait for JCP (or I don't want
> >> to),
> >> MK>> prototype some improvements in sandbox, for example:
> >> MK>>
> >> MK>> 1) we can create MyFacesExpressionFactory with methods
> >> MK>> createTagValueExpression, createLocationValueExpression,
> >> MK>> createResourceLocationValueExpression
> >> MK>>
> >>
> >> The problem here is the hacks required to make #{cc} and resource
> >> "this" are too myfaces specific, and are too coupled to myfaces impl.
> >>
> >> MK>> 2) In myfaces-core-impl then create default implementation with
> same
> >> MK>> code as TagAttributeImpl.getValueExpression(FaceletContext, Class)
> >> has
> >> MK>> currently.
> >> MK>>
> >>
> >> It could be good to have a factory pattern applied to that part.
> >>
> >> MK>> 3) create new module myfaces-integration with additional
> >> implementations
> >> MK>> of MyFacesExpressionFactory. For example
> >> JUELOWBMyFacesExpressionFactory
> >> MK>> - create* methods of such implementation will create not wrapped
> >> MK>> expression but one instance of JUELCODIValueExpression.
> >> MK>> JUELCODIValueExpression will use inheritance from JUEL internal API
> >> (and
> >> MK>> some code from OWB).
> >> MK>>
> >> MK>> Of course it will need cooperation with JUEL/OW
Re: [core] performance: avoid wrapped EL Expressions
On Sun, Jun 5, 2011 at 9:35 AM, Martin Koci
wrote:
> Hi,
>
> what problem is it? I know about excessive "rendered" evaluation:
> JAVASERVERFACES_SPEC_PUBLIC-941. "value" at EditableValueHolder can be
> evaluated 2x during one request/response.
>
> If it is another problem, can you create a jira issue, please?
>
I'm just talking about the number of times a getter is accessed per request
(when a value is bound to the value attribute), which can be several times.
It's not a bug per-se, just a side-effect of the spec/implementations.
>
>
> Thanks,
>
> Kočičák
>
> Kito Mann píše v Pá 03. 06. 2011 v 17:58 -0400:
> > Leonardo, would this reduce the number of calls to getters during
> > request processing?
> > ---
> > Kito D. Mann | twitter: kito99 | Author, JSF in Action
> > Virtua, Inc. | http://www.virtua.com | JSF/Java EE training and
> > consulting
> > http://www.JSFCentral.com - JavaServer Faces FAQ, news, and info |
> > twitter: jsfcentral
> > +1 203-404-4848 x3
> >
> > * Listen to the latest headlines in the JSF and Java EE
> > newscast: http://blogs.jsfcentral.com/roller/editorsdesk/category/JSF
> > +and+Java+EE+Newscast
> > * See you at JAX and JSF Summit 2011 June 20-23rd in San
> > Jose: http://jaxconf.com/
> >
> >
> > On Thu, Jun 2, 2011 at 10:10 PM, Leonardo Uribe
> > wrote:
> > Hi
> >
> > I have attached another patch to MYFACES-3160. This one has
> > the
> > ability to detect if the expression uses some variable
> > resolved by
> > facelets VariableMapper wrappers and if so, indicate the
> > expression
> > cannot be cached.
> >
> > This solution will work better, because it only creates
> > Value/Method
> > expressions when it is necessary. This is a great
> > optimization,
> > because in practice most expressions are bound to managed
> > beans, so in
> > practice it will make pages render faster (because EL parsing
> > is only
> > done once and a lot of objects will not be created) and the
> > memory
> > usage will be lower (less instances means gc works less).
> >
> > The only part this does not have any effect is when there is a
> > ValueExpression directly on the markup. In this case, I think
> > since
> > org.apache.myfaces.view.facelets.compiler.UIInstructionHandler
> > is
> > final (that means it is inmutable), put a volatile variable
> > for cache
> > such cases will make it a mutable class, so it cannot take
> > advantage
> > of some compiler optimizations. Maybe we can create two
> > classes, one
> > to handle markup without EL and other with EL. Anyway, the
> > most
> > critical point is expressions on attributes (changes on
> > facelets
> > compiler has to be done very carefully).
> >
> > JK> I would really like to see some prototyping work for JSF
> > 2.2 in this
> > JK> area directly in a MyFaces core branch!
> >
> > The patch proposed on MYFACES-3160 is for MyFaces 2.0 and 2.1.
> > After
> > the latest patch I think we don't really need some work on a
> > EL
> > implementation (see explanations below).
> >
> > MK>>
> > MK>> we need to avoid unnecessary ValueExpression instances.
> >
> > Yes, sure!. I know this optimization will be very useful, and
> > it will
> > do better use of available resource (memory and cpu).
> >
> > MK>>
> > MK>> Here is one idea: because we cannot wait for JCP (or I
> > don't want to),
> > MK>> prototype some improvements in sandbox, for example:
> > MK>>
> > MK>> 1) we can create MyFacesExpressionFactory with methods
> > MK>> createTagValueExpression, createLocationValueExpression,
> > MK>> createResourceLocationValueExpression
> > MK>>
> >
> > The problem here is the hacks required to make #{cc} and
> > resource
> > "this" are too myfaces specific, and are too coupled to
> > myfaces impl.
> >
> > MK>> 2) In myfaces-core-impl then create default
> > implementation with same
> > MK>> code as
> > TagAttributeImpl.getValueExpression(FaceletContext, Class) has
> > MK>> currently.
> > MK>>
> >
> > It could be good to have a factory pattern applied to that
> > part.
> >
> > MK>> 3) create new module myfaces-integration with additional
> > implementations
> > MK>> of MyFacesExpressionFactory. For example
> > JUELOWBMyFacesExpressionFactory
> > MK>> - create* methods of such implementation will create not
> > wrapped
> > MK>> expression but one instance of JUELCODIValueExpression.
> > MK>> JUELCODIValueExpression will use inheritance from JUEL
> >
Re: [core] performance: avoid wrapped EL Expressions
Hi,
what problem is it? I know about excessive "rendered" evaluation:
JAVASERVERFACES_SPEC_PUBLIC-941. "value" at EditableValueHolder can be
evaluated 2x during one request/response.
If it is another problem, can you create a jira issue, please?
Thanks,
Kočičák
Kito Mann píše v Pá 03. 06. 2011 v 17:58 -0400:
> Leonardo, would this reduce the number of calls to getters during
> request processing?
> ---
> Kito D. Mann | twitter: kito99 | Author, JSF in Action
> Virtua, Inc. | http://www.virtua.com | JSF/Java EE training and
> consulting
> http://www.JSFCentral.com - JavaServer Faces FAQ, news, and info |
> twitter: jsfcentral
> +1 203-404-4848 x3
>
> * Listen to the latest headlines in the JSF and Java EE
> newscast: http://blogs.jsfcentral.com/roller/editorsdesk/category/JSF
> +and+Java+EE+Newscast
> * See you at JAX and JSF Summit 2011 June 20-23rd in San
> Jose: http://jaxconf.com/
>
>
> On Thu, Jun 2, 2011 at 10:10 PM, Leonardo Uribe
> wrote:
> Hi
>
> I have attached another patch to MYFACES-3160. This one has
> the
> ability to detect if the expression uses some variable
> resolved by
> facelets VariableMapper wrappers and if so, indicate the
> expression
> cannot be cached.
>
> This solution will work better, because it only creates
> Value/Method
> expressions when it is necessary. This is a great
> optimization,
> because in practice most expressions are bound to managed
> beans, so in
> practice it will make pages render faster (because EL parsing
> is only
> done once and a lot of objects will not be created) and the
> memory
> usage will be lower (less instances means gc works less).
>
> The only part this does not have any effect is when there is a
> ValueExpression directly on the markup. In this case, I think
> since
> org.apache.myfaces.view.facelets.compiler.UIInstructionHandler
> is
> final (that means it is inmutable), put a volatile variable
> for cache
> such cases will make it a mutable class, so it cannot take
> advantage
> of some compiler optimizations. Maybe we can create two
> classes, one
> to handle markup without EL and other with EL. Anyway, the
> most
> critical point is expressions on attributes (changes on
> facelets
> compiler has to be done very carefully).
>
> JK> I would really like to see some prototyping work for JSF
> 2.2 in this
> JK> area directly in a MyFaces core branch!
>
> The patch proposed on MYFACES-3160 is for MyFaces 2.0 and 2.1.
> After
> the latest patch I think we don't really need some work on a
> EL
> implementation (see explanations below).
>
> MK>>
> MK>> we need to avoid unnecessary ValueExpression instances.
>
> Yes, sure!. I know this optimization will be very useful, and
> it will
> do better use of available resource (memory and cpu).
>
> MK>>
> MK>> Here is one idea: because we cannot wait for JCP (or I
> don't want to),
> MK>> prototype some improvements in sandbox, for example:
> MK>>
> MK>> 1) we can create MyFacesExpressionFactory with methods
> MK>> createTagValueExpression, createLocationValueExpression,
> MK>> createResourceLocationValueExpression
> MK>>
>
> The problem here is the hacks required to make #{cc} and
> resource
> "this" are too myfaces specific, and are too coupled to
> myfaces impl.
>
> MK>> 2) In myfaces-core-impl then create default
> implementation with same
> MK>> code as
> TagAttributeImpl.getValueExpression(FaceletContext, Class) has
> MK>> currently.
> MK>>
>
> It could be good to have a factory pattern applied to that
> part.
>
> MK>> 3) create new module myfaces-integration with additional
> implementations
> MK>> of MyFacesExpressionFactory. For example
> JUELOWBMyFacesExpressionFactory
> MK>> - create* methods of such implementation will create not
> wrapped
> MK>> expression but one instance of JUELCODIValueExpression.
> MK>> JUELCODIValueExpression will use inheritance from JUEL
> internal API (and
> MK>> some code from OWB).
> MK>>
> MK>> Of course it will need cooperation with JUEL/OWB
> developers (for example
> MK>> because de.odysseus.el.TreeValueExpression from JUEL is
> final class).
> MK>> Solution with inheritance is proposal only, I didn't try
> it yet. User
> MK>> must be sure that no other
Re: [core] performance: avoid wrapped EL Expressions
Hi,
Leonardo Uribe píše v Pá 03. 06. 2011 v 15:38 -0500:
> Hi
>
> 2011/6/3 Martin Koci :
> > Hi,
> >
> > the idea seems very good - but when is decided that expression does not
> > uses some variable resolved by VariableResolver?
>
> Inside VariableMapperWrapper.resolveVariable. If it returns a not null
> value, a variable has been resolved.
>
> > TagAttributeImpl.getValue/MethodExpression methods are called in
> > VDL.buildView - how is the check done? String parsing? Yes, I didn't
> > look in the patch, its friday :)
>
> No parsing is necessary.
I didn't realize that ExpressionFactory.createValueExpression calls
VariableMapper.resolveVariable. But it have to, because (from JavaDoc) "
The object returned must access the same variable mappings regardless of
whether the mappings in the provided FunctionMapper and VariableMapper
instances change between calling
ExpressionFactory.createValueExpression() and any method on
ValueExpression"
I tried that patch and it is very effective: it reduces response time
significantly, in one test case even about ~350ms. +1 for this solution.
Regards,
Kočičák
> >
> > Another idea: during VLD.buildView handler calls
> > TagAttribute.getMethodExpression and populates UIComponent with it. But
> > with partial lifecycle you don't need them all: with execute="@this" and
> > render="something" others components are untouched during lifecycle. Can
> > we create and use some kind o LazyValueExpression which parses and
> > initializes expression at first access?
> >
>
> The problem here is the context. As soon as facelets has built the
> view, VariableMapper info is lost, so on such LazyValueExpression you
> need to store that information (how?). Other problem is
> FunctionMapper, because it is setup "per page" so as soon as the page
> is processed, that context is lost. I don't think it could work. I
> think the strategy proposed here is better, because all Value/Method
> expressions are on Facelets Abstract Syntax Tree (AST), so once
> created it lives as long as that Facelet lives (see
> javax.faces.FACELETS_REFRESH_PERIOD). So, ajax operations will not
> recreate EL expressions if is not necessary.
>
> regards,
>
> Leonardo
>
> > Regards,
> >
> > Kočičák
> >
> > Leonardo Uribe píše v Čt 02. 06. 2011 v 21:10 -0500:
> >> Hi
> >>
> >> I have attached another patch to MYFACES-3160. This one has the
> >> ability to detect if the expression uses some variable resolved by
> >> facelets VariableMapper wrappers and if so, indicate the expression
> >> cannot be cached.
> >>
> >> This solution will work better, because it only creates Value/Method
> >> expressions when it is necessary. This is a great optimization,
> >> because in practice most expressions are bound to managed beans, so in
> >> practice it will make pages render faster (because EL parsing is only
> >> done once and a lot of objects will not be created) and the memory
> >> usage will be lower (less instances means gc works less).
> >>
> >> The only part this does not have any effect is when there is a
> >> ValueExpression directly on the markup. In this case, I think since
> >> org.apache.myfaces.view.facelets.compiler.UIInstructionHandler is
> >> final (that means it is inmutable), put a volatile variable for cache
> >> such cases will make it a mutable class, so it cannot take advantage
> >> of some compiler optimizations. Maybe we can create two classes, one
> >> to handle markup without EL and other with EL. Anyway, the most
> >> critical point is expressions on attributes (changes on facelets
> >> compiler has to be done very carefully).
> >>
> >> JK> I would really like to see some prototyping work for JSF 2.2 in this
> >> JK> area directly in a MyFaces core branch!
> >>
> >> The patch proposed on MYFACES-3160 is for MyFaces 2.0 and 2.1. After
> >> the latest patch I think we don't really need some work on a EL
> >> implementation (see explanations below).
> >>
> >> MK>>
> >> MK>> we need to avoid unnecessary ValueExpression instances.
> >>
> >> Yes, sure!. I know this optimization will be very useful, and it will
> >> do better use of available resource (memory and cpu).
> >>
> >> MK>>
> >> MK>> Here is one idea: because we cannot wait for JCP (or I don't want to),
> >> MK>> prototype some improvements in sandbox, for example:
> >> MK>>
> >> MK>> 1) we can create MyFacesExpressionFactory with methods
> >> MK>> createTagValueExpression, createLocationValueExpression,
> >> MK>> createResourceLocationValueExpression
> >> MK>>
> >>
> >> The problem here is the hacks required to make #{cc} and resource
> >> "this" are too myfaces specific, and are too coupled to myfaces impl.
> >>
> >> MK>> 2) In myfaces-core-impl then create default implementation with same
> >> MK>> code as TagAttributeImpl.getValueExpression(FaceletContext, Class) has
> >> MK>> currently.
> >> MK>>
> >>
> >> It could be good to have a factory pattern applied to that part.
> >>
> >> MK>> 3) create new module myfaces-integration wit
