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
