You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@myfaces.apache.org by Martin Koci <ma...@gmail.com> on 2011/06/02 20:22:27 UTC
[core] performance: avoid wrapped EL Expressions
Hi,
as discussed here:
http://markmail.org/message/kca64ojdvb6em367?q=[core]+performance:
+TagAttributeImpl+part+II:+object+allocations#query:[core]%20performance
%3A%20TagAttributeImpl%20part%20II%3A%20object%20allocations+page:1
+mid:kca64ojdvb6em367+state:results
and here:
http://markmail.org/search/?q=TagAttributeImpl+part+II%3A+object
+allocations+%28cache+ELExpressions%29#query:TagAttributeImpl%20part%
20II%3A%20object%20allocations%20%28cache%20ELExpressions%29%20from%3A%
22Leonardo%20Uribe%22+page:1+mid:pmurllp3w73q6c6s+state:results
we need to avoid unnecessary ValueExpression instances.
Here is one idea: because we cannot wait for JCP (or I don't want to),
prototype some improvements in sandbox, for example:
1) we can create MyFacesExpressionFactory with methods
createTagValueExpression, createLocationValueExpression,
createResourceLocationValueExpression ....
2) In myfaces-core-impl then create default implementation with same
code as TagAttributeImpl.getValueExpression(FaceletContext, Class) has
currently.
3) create new module myfaces-integration with additional implementations
of MyFacesExpressionFactory. For example JUELOWBMyFacesExpressionFactory
- create* methods of such implementation will create not wrapped
expression but one instance of JUELCODIValueExpression.
JUELCODIValueExpression will use inheritance from JUEL internal API (and
some code from OWB).
Of course it will need cooperation with JUEL/OWB developers (for example
because de.odysseus.el.TreeValueExpression from JUEL is final class).
Solution with inheritance is proposal only, I didn't try it yet. User
must be sure that no other library wants to wrap ValueExpression.
WDYT?
Kočičák
Re: [core] performance: avoid wrapped EL Expressions
Posted by Martin Koci <ma...@gmail.com>.
Hi,
Leonardo Uribe píše v Pá 03. 06. 2011 v 15:38 -0500:
> Hi
>
> 2011/6/3 Martin Koci <ma...@gmail.com>:
> > 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 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 library wants to wrap ValueExpression.
> >> MK>>
> >>
> >> In my mind this only works as a "last resource". VariableMapper usage
> >> is only a concern inside facelets, because its usage is bound to the
> >> context the expression is created.
> >>
> >> Anyway, I would like to know first if it is really necessary to create
> >> such factories. We need concrete use cases that support that. For now,
> >> I'll be happy with the solution proposed. It still requires a deep
> >> review (because the code affected is very critical) and some junit
> >> tests, so it will take some time before finish it.
> >>
> >> regards,
> >>
> >> Leonardo Uribe
> >>
> >
> >
> >
>
Re: [core] performance: avoid wrapped EL Expressions
Posted by Leonardo Uribe <lu...@gmail.com>.
Hi
2011/6/3 Martin Koci <ma...@gmail.com>:
> 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.
>
> 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 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 library wants to wrap ValueExpression.
>> MK>>
>>
>> In my mind this only works as a "last resource". VariableMapper usage
>> is only a concern inside facelets, because its usage is bound to the
>> context the expression is created.
>>
>> Anyway, I would like to know first if it is really necessary to create
>> such factories. We need concrete use cases that support that. For now,
>> I'll be happy with the solution proposed. It still requires a deep
>> review (because the code affected is very critical) and some junit
>> tests, so it will take some time before finish it.
>>
>> regards,
>>
>> Leonardo Uribe
>>
>
>
>
Re: [core] performance: avoid wrapped EL Expressions
Posted by Martin Koci <ma...@gmail.com>.
Hi,
the idea seems very good - but when is decided that expression does not
uses some variable resolved by VariableResolver?
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 :)
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?
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 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 library wants to wrap ValueExpression.
> MK>>
>
> In my mind this only works as a "last resource". VariableMapper usage
> is only a concern inside facelets, because its usage is bound to the
> context the expression is created.
>
> Anyway, I would like to know first if it is really necessary to create
> such factories. We need concrete use cases that support that. For now,
> I'll be happy with the solution proposed. It still requires a deep
> review (because the code affected is very critical) and some junit
> tests, so it will take some time before finish it.
>
> regards,
>
> Leonardo Uribe
>
Re: [core] performance: avoid wrapped EL Expressions
Posted by Leonardo Uribe <lu...@gmail.com>.
Hi
Checking this patch I founded one use case where the hack proposed could fail:
<c:if test="#{somecondition}">
<c:set var="myvar" value="somevalue"/>
</c:if>
<!-- code that use myvar -->
I found it in this link:
http://seamframework.org/Documentation/CreatingACompositeAjaxifiedForm
In this case, things could be rewritten to this:
<c:set var="myvar" value="#{somecondition ? 'somevalue' : null}"/>
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
<ui:decorate template="cset2_1.xhtml">
</ui:decorate>
<h:outputText value="#{pages}"/>
cset2_1.xhtml
<ui:composition>
<c:set var="pages" value="20"/>
</ui:composition>
#{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
<c:set var="pages" value="20"/>
<ui:decorate template="cset2_1.xhtml">
</ui:decorate>
cset3_1.xhtml
<ui:composition>
<h:outputText value="#{pages}"/>
</ui:composition>
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 <ki...@virtua.com>:
> 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 <lu...@gmail.com> 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 <ki...@virtua.com>:
>> > 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 <lu...@gmail.com>
>> > 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 library wants to wrap ValueExpression.
>> >> MK>>
>> >>
>> >> In my mind this only works as a "last resource". VariableMapper usage
>> >> is only a concern inside facelets, because its usage is bound to the
>> >> context the expression is created.
>> >>
>> >> Anyway, I would like to know first if it is really necessary to create
>> >> such factories. We need concrete use cases that support that. For now,
>> >> I'll be happy with the solution proposed. It still requires a deep
>> >> review (because the code affected is very critical) and some junit
>> >> tests, so it will take some time before finish it.
>> >>
>> >> regards,
>> >>
>> >> Leonardo Uribe
>> >
>> >
>
>
Re: [core] performance: avoid wrapped EL Expressions
Posted by Kito Mann <ki...@virtua.com>.
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 <lu...@gmail.com> 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 <ki...@virtua.com>:
> > 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 <lu...@gmail.com>
> 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 library wants to wrap ValueExpression.
> >> MK>>
> >>
> >> In my mind this only works as a "last resource". VariableMapper usage
> >> is only a concern inside facelets, because its usage is bound to the
> >> context the expression is created.
> >>
> >> Anyway, I would like to know first if it is really necessary to create
> >> such factories. We need concrete use cases that support that. For now,
> >> I'll be happy with the solution proposed. It still requires a deep
> >> review (because the code affected is very critical) and some junit
> >> tests, so it will take some time before finish it.
> >>
> >> regards,
> >>
> >> Leonardo Uribe
> >
> >
>
Re: [core] performance: avoid wrapped EL Expressions
Posted by Leonardo Uribe <lu...@gmail.com>.
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 <ki...@virtua.com>:
> 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 <lu...@gmail.com> 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 library wants to wrap ValueExpression.
>> MK>>
>>
>> In my mind this only works as a "last resource". VariableMapper usage
>> is only a concern inside facelets, because its usage is bound to the
>> context the expression is created.
>>
>> Anyway, I would like to know first if it is really necessary to create
>> such factories. We need concrete use cases that support that. For now,
>> I'll be happy with the solution proposed. It still requires a deep
>> review (because the code affected is very critical) and some junit
>> tests, so it will take some time before finish it.
>>
>> regards,
>>
>> Leonardo Uribe
>
>
Re: [core] performance: avoid wrapped EL Expressions
Posted by Kito Mann <ki...@virtua.com>.
On Sun, Jun 5, 2011 at 9:35 AM, Martin Koci
<ma...@gmail.com>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 <lu...@gmail.com>
> > 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 library wants to wrap
> > ValueExpression.
> > MK>>
> >
> > In my mind this only works as a "last resource".
> > VariableMapper usage
> > is only a concern inside facelets, because its usage is bound
> > to the
> > context the expression is created.
> >
> > Anyway, I would like to know first if it is really necessary
> > to create
> > such factories. We need concrete use cases that support that.
> > For now,
> > I'll be happy with the solution proposed. It still requires a
> > deep
> > review (because the code affected is very critical) and some
> > junit
> > tests, so it will take some time before finish it.
> >
> > regards,
> >
> > Leonardo Uribe
> >
>
>
>
Re: [core] performance: avoid wrapped EL Expressions
Posted by Martin Koci <ma...@gmail.com>.
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 <lu...@gmail.com>
> 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 library wants to wrap
> ValueExpression.
> MK>>
>
> In my mind this only works as a "last resource".
> VariableMapper usage
> is only a concern inside facelets, because its usage is bound
> to the
> context the expression is created.
>
> Anyway, I would like to know first if it is really necessary
> to create
> such factories. We need concrete use cases that support that.
> For now,
> I'll be happy with the solution proposed. It still requires a
> deep
> review (because the code affected is very critical) and some
> junit
> tests, so it will take some time before finish it.
>
> regards,
>
> Leonardo Uribe
>
Re: [core] performance: avoid wrapped EL Expressions
Posted by Kito Mann <ki...@virtua.com>.
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 <lu...@gmail.com> 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 library wants to wrap ValueExpression.
> MK>>
>
> In my mind this only works as a "last resource". VariableMapper usage
> is only a concern inside facelets, because its usage is bound to the
> context the expression is created.
>
> Anyway, I would like to know first if it is really necessary to create
> such factories. We need concrete use cases that support that. For now,
> I'll be happy with the solution proposed. It still requires a deep
> review (because the code affected is very critical) and some junit
> tests, so it will take some time before finish it.
>
> regards,
>
> Leonardo Uribe
>
Re: [core] performance: avoid wrapped EL Expressions
Posted by Leonardo Uribe <lu...@gmail.com>.
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 library wants to wrap ValueExpression.
MK>>
In my mind this only works as a "last resource". VariableMapper usage
is only a concern inside facelets, because its usage is bound to the
context the expression is created.
Anyway, I would like to know first if it is really necessary to create
such factories. We need concrete use cases that support that. For now,
I'll be happy with the solution proposed. It still requires a deep
review (because the code affected is very critical) and some junit
tests, so it will take some time before finish it.
regards,
Leonardo Uribe
Re: [core] performance: avoid wrapped EL Expressions
Posted by Jakob Korherr <ja...@gmail.com>.
Hi Martin,
Good idea! +1 from my side.
Unfortunately, EL is one of the most expensive things in JSF and
wrapping Value-/MethodExpressions does not make it better!
I would really like to see some prototyping work for JSF 2.2 in this
area directly in a MyFaces core branch!
Regards,
Jakob
2011/6/2 Martin Koci <ma...@gmail.com>:
> Hi,
>
> as discussed here:
> http://markmail.org/message/kca64ojdvb6em367?q=[core]+performance:
> +TagAttributeImpl+part+II:+object+allocations#query:[core]%20performance
> %3A%20TagAttributeImpl%20part%20II%3A%20object%20allocations+page:1
> +mid:kca64ojdvb6em367+state:results
>
> and here:
>
> http://markmail.org/search/?q=TagAttributeImpl+part+II%3A+object
> +allocations+%28cache+ELExpressions%29#query:TagAttributeImpl%20part%
> 20II%3A%20object%20allocations%20%28cache%20ELExpressions%29%20from%3A%
> 22Leonardo%20Uribe%22+page:1+mid:pmurllp3w73q6c6s+state:results
>
> we need to avoid unnecessary ValueExpression instances.
>
> Here is one idea: because we cannot wait for JCP (or I don't want to),
> prototype some improvements in sandbox, for example:
>
> 1) we can create MyFacesExpressionFactory with methods
> createTagValueExpression, createLocationValueExpression,
> createResourceLocationValueExpression ....
>
> 2) In myfaces-core-impl then create default implementation with same
> code as TagAttributeImpl.getValueExpression(FaceletContext, Class) has
> currently.
>
> 3) create new module myfaces-integration with additional implementations
> of MyFacesExpressionFactory. For example JUELOWBMyFacesExpressionFactory
> - create* methods of such implementation will create not wrapped
> expression but one instance of JUELCODIValueExpression.
> JUELCODIValueExpression will use inheritance from JUEL internal API (and
> some code from OWB).
>
> Of course it will need cooperation with JUEL/OWB developers (for example
> because de.odysseus.el.TreeValueExpression from JUEL is final class).
> Solution with inheritance is proposal only, I didn't try it yet. User
> must be sure that no other library wants to wrap ValueExpression.
>
>
> WDYT?
>
> Kočičák
>
>
--
Jakob Korherr
blog: http://www.jakobk.com
twitter: http://twitter.com/jakobkorherr
work: http://www.irian.at