Ah yes. The assembler == null bit was leftover from my previous
implementation (where assemblers were cached instead of Class
instances). I will correct this and send in that patch along with a
patch to changes.xml.
-B
On Jul 8, 2004, at 2:52 AM, Henning P. Schmiedehausen wrote:
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
