You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by bu...@apache.org on 2012/03/28 23:16:03 UTC

DO NOT REPLY [Bug 52998] New: Performance issue with ExpressionFactory.newInstance()

https://issues.apache.org/bugzilla/show_bug.cgi?id=52998

             Bug #: 52998
           Summary: Performance issue with ExpressionFactory.newInstance()
           Product: Tomcat 7
           Version: 7.0.26
          Platform: PC
        OS/Version: Windows XP
            Status: NEW
          Severity: normal
          Priority: P2
         Component: Jasper
        AssignedTo: dev@tomcat.apache.org
        ReportedBy: knst.kolinko@gmail.com
    Classification: Unclassified


javax.el.ExpressionFactory.newInstance() implementation in Tomcat 7 does not
cache created instance and performs class name discovery on every invocation.

The discovery process involves looking for the file named
"META-INF/services/javax.el.ExpressionFactory". So every invocation of the
method involves looking for and maybe reading the file.


This issue was reported on the dev list, see thread:

[1] "Two performance problems (found during myfaces testing)", starting on
2012-03-08,
- http://tomcat.markmail.org/thread/7bbvzmkvyvryvn44
- http://marc.info/?t=133124021200002&r=1&w=2

The above thread [1] also references this one of myfaces:
[2] "EL method invocation performance", 2010-08-25:
- http://www.mail-archive.com/dev@myfaces.apache.org/msg48482.html

------------------
My evaluation is that
1. This problem is specific for Tomcat 7. - The ExpressionFactory.newInstance()
method was added in EL 2.2 and Tomcat 6 does not have it.

2. It hits javax.el.BeanELResolver#invoke() the most, as
ExpressionFactory.newInstance() is called on each invocation.

3. There are 2 places where the factory instance is stored in a static field.
This is good for performance, but breaks the discovery process and may cause
consequences if implementation is bundled in a web application. It is a bug.
The places:

 org.apache.jasper.runtime.JspApplicationContextImpl.expressionFactory
 org.apache.jasper.compiler.Validator$ValidateVisitor.EXPRESSION_FACTORY

-- 
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org

DO NOT REPLY [Bug 52998] Performance issue with ExpressionFactory.newInstance()

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=52998

Mark Thomas <ma...@apache.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED

--- Comment #3 from Mark Thomas <ma...@apache.org> 2012-03-30 20:12:54 UTC ---
Patch looks good to me. I applied it so I can get on with the pre-release
testing.

-- 
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org

DO NOT REPLY [Bug 52998] Performance issue with ExpressionFactory.newInstance()

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=52998

--- Comment #1 from Konstantin Kolinko <kn...@gmail.com> 2012-03-30 10:15:35 UTC ---
Created attachment 28518
  --> https://issues.apache.org/bugzilla/attachment.cgi?id=28518
2012-03-30_tc8_52998_ExpressionFactory.patch

Patch against trunk that implements caching in ExpressionFactory.

Notes:
1. r1307310 should be ported to 7.0.x before this patch.

Maybe whitespace cleanup r1187778 shall be ported as well.

2. This patch only implements the caching.

The issue with static references mentioned in Comment 0 is not addressed.

3. There is no way to clean stale cache entries from stopped webapps,
but the cache entry objects are small, so I think we can live with it.

Weak references are used, so the class loaders from undeployed apps should be
eligible for garbage collection.

-- 
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org

DO NOT REPLY [Bug 52998] Performance issue with ExpressionFactory.newInstance()

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=52998

--- Comment #2 from Mark Thomas <ma...@apache.org> 2012-03-30 19:34:35 UTC ---
I've taken care of the static references in Jasper. I haven't reviewed the
patch for the remaining issues yet.

-- 
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org