You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@myfaces.apache.org by Martin Koci <ma...@gmail.com> on 2010/10/21 21:45:46 UTC
[core] performance: TagAttributeImpl part II: object allocations
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
Re: [core] performance: TagAttributeImpl part II: object allocations
Posted by Leonardo Uribe <lu...@gmail.com>.
Hi
2010/10/25 Martin Koci <ma...@gmail.com>
> 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 <ja...@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 <lu...@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 <ja...@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 <ma...@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
> >
> >
>
>
>
Re: [core] performance: TagAttributeImpl part II: object
allocations
Posted by Martin Koci <ma...@gmail.com>.
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.
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.
>
> Yes, we should test that possible optimization well.
>
> regards,
>
> Leonardo Uribe
>
> 2010/10/21 Jakob Korherr <ja...@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 <lu...@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 <ja...@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 <ma...@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
>
>
Re: [core] performance: TagAttributeImpl part II: object allocations
Posted by Leonardo Uribe <lu...@gmail.com>.
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 <ja...@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 <lu...@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 <ja...@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 <ma...@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
>
Re: [core] performance: TagAttributeImpl part II: object allocations
Posted by Jakob Korherr <ja...@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 <lu...@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 <ja...@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 <ma...@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
Re: [core] performance: TagAttributeImpl part II: object allocations
Posted by Leonardo Uribe <lu...@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 <ja...@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 <ma...@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
>
Re: [core] performance: TagAttributeImpl part II: object allocations
Posted by Jakob Korherr <ja...@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 <ma...@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