myfaces-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Leonardo Uribe <lu4...@gmail.com>
Subject Re: [core] performance: avoid wrapped EL Expressions
Date Fri, 03 Jun 2011 20:38:49 GMT
Hi

2011/6/3 Martin Koci <martin.kocicak.koci@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
>>
>
>
>

Mime
View raw message