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: TagAttributeImpl part II: object allocations
Date Fri, 22 Oct 2010 01:31:56 GMT
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, we should test that possible optimization well.

regards,

Leonardo Uribe

2010/10/21 Jakob Korherr <jakob.korherr@gmail.com>

> 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 <lu4242@gmail.com>:
> > 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 <jakob.korherr@gmail.com>
> >>
> >> 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 <martin.kocicak.koci@gmail.com>:
> >> > 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
>

Mime
View raw message