Brian Lawler <[EMAIL PROTECTED]> writes:
>So at long last, here is the patch for JavaBaseFactory, for you perusal
>(again). Just to refresh the context, the problem that we ran into on
>our site is that excessive calls to Class.forName() in the
>JavaBaseFactory was bubbling up to the top of our profiler output.
>Turns out that the Base Factory was repeatedly calling Class.forName
>for the same class name. This is easily remedied by just creating a
>Map of Class instances in the JavaBaseFactory class.
Hi,
looks ok to me. I do consider applying it. Please add a patch to
xdocs/changes.xml which describes this change and the reason why.
Why are you adding the assembler == null to the for() loop? When the
assembler gets set, the following break breaks the for() loop anyway,
so this test is always true.
Regards
Henning
>Here is the patch, applied against TURBINE_2_3_BRANCH:
>Index:
>src/java/org/apache/turbine/services/assemblerbroker/util/java/
>JavaBaseFactory.java
>===================================================================
>RCS file:
>/home/cvspublic/jakarta-turbine-2/src/java/org/apache/turbine/services/
>assemblerbroker/util/java/JavaBaseFactory.java,v
>retrieving revision 1.8.2.2
>diff -u -r1.8.2.2 JavaBaseFactory.java
>---
>src/java/org/apache/turbine/services/assemblerbroker/util/java/
>JavaBaseFactory.java 20 May 2004 03:05:16 -0000 1.8.2.2
>+++
>src/java/org/apache/turbine/services/assemblerbroker/util/java/
>JavaBaseFactory.java 7 Jul 2004 18:13:17 -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;
>@@ -35,9 +38,7 @@
> * A screen factory that attempts to load a java class from
> * the module packages defined in the TurbineResource.properties.
> *
>- * @author <a href="mailto:[EMAIL PROTECTED]">Leon Messerschmidt</a>
>- * @author <a href="mailto:[EMAIL PROTECTED]">Henning P.
>Schmiedehausen</a>
>- * @version $Id: JavaBaseFactory.java,v 1.8.2.2 2004/05/20 03:05:16
>seade Exp $
>+ * @version $Id: JavaBaseFactory.java,v 1.2 2004/07/07 17:45:05 brian
>Exp $
> */
> public abstract class JavaBaseFactory
> implements AssemblerFactory
>@@ -49,6 +50,12 @@
> /** Logging */
> protected Log log = LogFactory.getLog(this.getClass());
>+ /**
>+ * A cache for previously obtained Class instances, which we keep
>in order
>+ * to reduce the Class.forName() overhead (which can be sizable).
>+ */
>+ private Map classCache = Collections.synchronizedMap(new
>HashMap());
>+
> static
> {
> ObjectUtils.addOnce(packages, GenericLoader.getBasePackage());
>@@ -69,7 +76,7 @@
> if (StringUtils.isNotEmpty(name))
> {
>- for (Iterator it = packages.iterator(); it.hasNext();)
>+ for (Iterator it = packages.iterator(); assembler == null
>&& it.hasNext();)
> {
> StringBuffer className = new StringBuffer();
>@@ -83,7 +90,12 @@
> try
> {
>- Class servClass =
>Class.forName(className.toString());
>+ Class servClass = (Class)
>classCache.get(className);
>+ if(servClass == null)
>+ {
>+ servClass =
>Class.forName(className.toString());
>+ classCache.put(className, servClass);
>+ }
> assembler = (Assembler) servClass.newInstance();
> break; // for()
> }
>On Jun 12, 2004, at 1:37 AM, Brian Lawler wrote:
>> Henning-
>>
>> On May 9, 2004, at 4:37 PM, Henning P. Schmiedehausen wrote:
>>>
>>> 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.
>>>
>>
>> Great point. I have re-implemented this as a Class cache, since
>> Class.forName() is indeed our bottleneck, and I will be testing it
>> over the next few days. I will send my patched patch when satisfied
>> that it is doing the right thing...
>>
>> -B
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: [EMAIL PROTECTED]
>> For additional commands, e-mail: [EMAIL PROTECTED]
>>
>---------------------------------------------------------------------
>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
"Fighting for one's political stand is an honourable action, but re-
fusing to acknowledge that there might be weaknesses in one's
position - in order to identify them so that they can be remedied -
is a large enough problem with the Open Source movement that it
deserves to be on this list of the top five problems."
-- Michelle Levesque, "Fundamental Issues with
Open Source Software Development"
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
