Hi
2010/10/25 Martin Koci <[email protected]>
> Leonardo Uribe píše v Čt 21. 10. 2010 v 20:31 -0500:
> > Hi
> >
> > I investigate more if it is possible and unfortunately it is not as
> > default. ValueExpressions
> > instances are immutable, but when
> > ExpressionFactory.createValueExpression is called,
> > this method uses FunctionMapper and VariableMapper provided by
> > FaceletContext.
> >
> > The problem is there is no way to detect if a ValueExpression was
> > constructed using
> > FunctionMapper or VariableMapper, and facelets uses them a lot.
> >
> > But in most cases, FunctionMapper and VariableMapper passed by
> > facelets does not change,
> > that means, no new variables of functions are added while facelet is
> > processing the same page
> > over and over. So cache them with a optional parameter (default false)
> > could work.
>
> Yes, especially if view declaration doesn't use build time tags like
> c:if, c:choose or some kind of direct manipulation of component
> tree/ELContext -> then variable mapping should be same for each request.
>
>
On myfaces there are these cases that set attributes to the VariableMapper
that could change on different requests:
- c:if var
- c:catch var
- c:forEach var and varStatus
- c:set
I think it is possible to do something in each case to just disable the
optimization,
so the instantiation is not cached an the affected code will work. There are
other
usages that instead :
- ui:param:
String nameStr = this.name.getValue(ctx);
ValueExpression valueVE = this.value.getValueExpression(ctx,
Object.class);
ctx.getVariableMapper().setVariable(nameStr, valueVE);
- user tag handler
VariableMapper varMapper = new VariableMapperWrapper(orig);
for (int i = 0; i < this._vars.length; i++)
{
varMapper.setVariable(this._vars[i].getLocalName(),
this._vars[i].getValueExpression(ctx, Object.class));
}
ctx.setVariableMapper(varMapper);
These are just not a problem because it set a a variable with a
ValueExpression that is created from the page, so if it is literal its
value does not change.
> EL specification directly says that: "Expressions are also designed to
> be immutable so that only one instance needs to be created for any given
> expression String / FunctionMapper. This allows a container to
> pre-create expressions and not have to reparse them each time they are
> evaluated" so the only problem here is really the variable mapping.
>
>
I took a look again, and I agree FunctionMapper is not a concern. The
problem is
VariableMapper usage, but anyway it could be a special optimization by
default false.
It looks promising.
regards,
Leonardo Uribe
> >
> > Yes, we should test that possible optimization well.
> >
> > regards,
> >
> > Leonardo Uribe
> >
> > 2010/10/21 Jakob Korherr <[email protected]>
> > Looks promising, but are they really considered immutable? If
> > we do
> > this change, we should test special scenarios with all
> > available EL
> > impls (GlassFish, Tomcat, Geronimo, JUEL) first, because the
> > behavior
> > might differ from impl to impl..
> >
> > Regards,
> > Jakob
> >
> > 2010/10/22 Leonardo Uribe <[email protected]>:
> >
> > > Hi
> > >
> > > In theory it is possible to cache ValueExpression creation
> > at
> > > TagAttributeImpl level, because ValueExpression instances
> > are
> > > immutable classes (once initialized does not change its
> > state)
> > > and Serializable.
> > >
> > > Just add a simple variable there could do the job. Just add
> > a variable
> > > and fill it when getValueExpression(FaceletContext, Class)
> > or
> > > getMethodExpression(FaceletContext, Class, Class[]) is being
> > > called. If two different threads fill this value at the same
> > time
> > > it will no matter, because both are the same.
> > >
> > > It is a hack very similar to
> > > CompositeComponentDefinitionTagHandler._cachedBeanInfo:
> > >
> > > /**
> > > * Cached instance used by this component. Note here we
> > have a
> > > * "racy single-check".If this field is used, it is
> > supposed
> > > * the object cached by this handler is immutable, and
> > this is
> > > * granted if all properties not saved as
> > ValueExpression are
> > > * "literal".
> > > **/
> > >
> > > What do you think guys?
> > >
> > > regards,
> > >
> > > Leonardo Uribe
> > >
> > > 2010/10/21 Jakob Korherr <[email protected]>
> > >>
> > >> Hi Martin,
> > >>
> > >> Yes, I totally agree. This is really a big performance
> > problem.
> > >>
> > >> The main problem here is that we have no real control about
> > the
> > >> creation of the ValueExpressions, because the EL
> > implementation
> > >> provides them for us, thus the wrapper approach.
> > >>
> > >> The first wrapper, TagValueExpression, (which is actually
> > used for
> > >> every facelet attribute ValueExpression, right?) might not
> > really be
> > >> necessary, I guess. The class comes directly from the
> > >> facelets-codebase, so we actually don't know why it was
> > introduced. I
> > >> haven't looked at it yet, but I don't think it has any
> > further sence.
> > >> Maybe we can get rid of this wrapper...
> > >>
> > >> The second wrapper is a must, otherwise composite component
> > EL
> > >> expressions (#{cc}) won't work as expected, because the
> > resolution
> > >> mechanism has to use the composite component from the xhtml
> > site on
> > >> which the ValueExpression is defined. However, those
> > ValueExpressions
> > >> are not used that much, I guess.
> > >>
> > >> Suggestions are welcome!
> > >>
> > >> Regards,
> > >> Jakob
> > >>
> > >> 2010/10/21 Martin Koci <[email protected]>:
> > >> > Hi,
> > >> >
> > >> > another performance related problem in TagAttributeImpl:
> > ValueExpression
> > >> > instances.
> > >> >
> > >> > Consider a facelets view with 3000 attributes. Then
> > during buildView
> > >> > method TagAttributeImpl.getValueExpression allocates:
> > >> >
> > >> > 1) one instance of ValueExpression via
> > >> > ExpressionFactory.createValueExpression
> > >> >
> > >> > 2) one instance of ValueExpression as TagValueExpression
> > >> >
> > >> > 3) if TagAttribute is inside composite component then
> > allocates another
> > >> > instance of LocationValueExpression.
> > >> >
> > >> >
> > >> > In this test case buildView creates at least 6 000
> > instances of
> > >> > ValueExpression. This is not problem because instances
> > are cheap and
> > >> > allocation doesn't affect CPU consumption. Problem
> > appears if more
> > >> > threads do the same: for 100 threads/requests it is 600
> > 000 instances;
> > >> > consider it for 1000 concurrent requests. All those
> > instances are very
> > >> > short-lived and practically never leave HotSpot's Young
> > Generation space
> > >> > and triggers GC excessively; GC management it the main
> > problem : after
> > >> > one hour of running stress test is
> > TagAttributeImpl.getValueExpression
> > >> > #1 in "hot spot by object count" with millions of
> > allocations.
> > >> >
> > >> > At first sight allocations 2) and 3) serves only as a
> > kind of
> > >> > TagAttribute <-> ValueExpression mapping. Specially the
> > second one holds
> > >> > only one String which serves later for a nicer exception
> > report.
> > >> >
> > >> > It seems that some better kind of TagAttribute <->
> > ValueExpression <->
> > >> > (String, Localtion) relation reprezentation without using
> > wrapper
> > >> > pattern can solve this problem. Any ideas how to do it?
> > >> >
> > >> >
> > >> > Regards,
> > >> >
> > >> >
> > >> > Kočičák
> > >> >
> > >> >
> > >> >
> > >> >
> > >> >
> > >> >
> > >> >
> > >> >
> > >>
> > >>
> > >>
> > >> --
> > >> Jakob Korherr
> > >>
> > >> blog: http://www.jakobk.com
> > >> twitter: http://twitter.com/jakobkorherr
> > >> work: http://www.irian.at
> > >
> > >
> >
> >
> >
> >
> > --
> >
> > Jakob Korherr
> >
> > blog: http://www.jakobk.com
> > twitter: http://twitter.com/jakobkorherr
> > work: http://www.irian.at
> >
> >
>
>
>
