Brian Lawler <[EMAIL PROTECTED]> writes:
>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.
Cool. Don't forget to add you to the contributors in project.xml :-)
I'll apply the patch then (unless someone objects).
Regards
Henning
>-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]
--
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 honorable 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]
