Hi, Leonardo Uribe píše v Pá 03. 06. 2011 v 15:38 -0500: > Hi > > 2011/6/3 Martin Koci <martin.kocicak.k...@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 > >> > > > > > > >