Brian Lawler <[EMAIL PROTECTED]> writes:
>Hey guys-
>We were profiling our turbine application the other day and one thing
>that bubbled up to the top was the JavaBaseFactory.getAssembler()
>class. This class iterates over all of the packages where Assemblers
>may be found and retrieves a Class instance when it encounters a hit.
>But it does this for every call to the getAssembler method, and we
>found the resulting time spent in Class.forName() to be very high. I
>have placed a simple synchronized map in our version of the class that
>is used to cache previously found Assemblers and is queried before
>going through the package loop. This lowered the time we spend in
>Class.forName() substantially.
>First of all, is this not a good thing to do?
Well, why not?
>And second, if this *is* a good thing to do, here is the patch if you
>are interested...
Did you try running a patched Turbine? Did it make any difference in
the loading process of the classes? I'm a little concerned about the
long-term life of these maps and their member objects. Currently, the
assemblers are created anew whenever one is required. With your patch,
there might be side effects with internal state that is now kept
because the objects are not created every time but kept around in a
table. I'm also concerned that these Factories, while being
instantiated are just one object (in the AssemblerBrokerService) for
the whole application with all the parallel running sessions. ATM,
each session instantiates its own Assembler whenever it needs one. Now
there is only one Assembler object per match for all the sessions
being pulled from a cache. This looks like a point where debugging
(having exactly one session which does one thing at a time) and real
life (having lots of sessions doing different things) don't match.
You might either want to have some sort of real life cycle (which
would be overkill for a point release like 2.3.1) or, if just
Class.forName() is your bottleneck, simply store the Class objects and
not the instance objects. This could even be a class cache, not just
an instance cache, because translating
"com.mycorp.modules.actions.FooAction" to a Class object describing
the FooAction class is the same for all the instances.
Regards
Henning
>Index:
>src/java/org/apache/turbine/services/assemblerbroker/util/java/
>JavaBaseFactory.java
>===================================================================
>RCS file:
>/home/cvsroot/turbine/src/java/org/apache/turbine/services/
>assemblerbroker/util/java/JavaBaseFactory.java,v
>retrieving revision 1.1.1.2
>diff -u -r1.1.1.2 JavaBaseFactory.java
>---
>src/java/org/apache/turbine/services/assemblerbroker/util/java/
>JavaBaseFactory.java 20 Apr 2004 23:59:23 -0000 1.1.1.2
>+++
>src/java/org/apache/turbine/services/assemblerbroker/util/java/
>JavaBaseFactory.java 21 Apr 2004 00:06:03 -0000
>@@ -16,7 +16,10 @@
> * limitations under the License.
> */
>+import java.util.Collections;
>+import java.util.HashMap;
> import java.util.Iterator;
>+import java.util.Map;
> import java.util.Vector;
> import org.apache.commons.lang.StringUtils;
>@@ -47,6 +50,9 @@
> /** Logging */
> protected Log log = LogFactory.getLog(this.getClass());
>+ /** A cache for previously obtained assemblers */
>+ private Map assemblerCache = Collections.synchronizedMap(new
>HashMap());
>+
> static
> {
> ObjectUtils.addOnce(packages, GenericLoader.getBasePackage());
>@@ -67,7 +73,8 @@
> if (StringUtils.isNotEmpty(name))
> {
>- for (Iterator it = packages.iterator(); it.hasNext();)
>+ assembler = (Assembler) assemblerCache.get(name);
>+ for (Iterator it = packages.iterator(); assembler == null
>&& it.hasNext();)
> {
> StringBuffer className = new StringBuffer();
>@@ -83,6 +90,7 @@
> {
> Class servClass =
>Class.forName(className.toString());
> assembler = (Assembler) servClass.newInstance();
>+ assemblerCache.put(name, assembler);
> break; // for()
> }
> catch (ClassNotFoundException cnfe)
>---------------------------------------------------------------------
>To unsubscribe, e-mail: [EMAIL PROTECTED]
>For additional commands, e-mail: [EMAIL PROTECTED]
--
Dipl.-Inf. (Univ.) Henning P. Schmiedehausen INTERMETA GmbH
[EMAIL PROTECTED] +49 9131 50 654 0 http://www.intermeta.de/
RedHat Certified Engineer -- Jakarta Turbine Development -- hero for hire
Linux, Java, perl, Solaris -- Consulting, Training, Development
"Au�erdem k�nnen in Deutschland alle Englisch. [...] so entf�llt die
Notwendigkeit [...] Deutsch zu lernen."
-- Johan Micoud auf die Frage warum er kein Deutsch spricht.
(http://www.spiegel.de/spiegel/0,1518,273205,00.html)
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
