You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by kev <ke...@neoapathy.org> on 2003/04/12 15:22:22 UTC
[patch]StandardContext general code cleanup and findWelcomeFiles alternate implementation
I've gone through most of the code in StandardContext and cleaned up
if/for {} to try and keep the code consistent across the class. I've
also changed findWelcomeFiles to remove the two returns from the method.
patch file below
---------------------
--- ./StandardContext.java Sat Apr 12 14:02:13 2003
+++ ./StandardContext-new.java Sat Apr 12 14:11:05 2003
@@ -66,6 +66,7 @@
import java.io.IOException;
import java.io.Serializable;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.HashMap;
import java.util.Iterator;
import java.util.TreeMap;
@@ -978,9 +979,10 @@
public void setLoginConfig(LoginConfig config) {
// Validate the incoming property value
- if (config == null)
+ if (config == null) {
throw new IllegalArgumentException
(sm.getString("standardContext.loginConfig.required"));
+ }
String loginPage = config.getLoginPage();
if ((loginPage != null) && !loginPage.startsWith("/")) {
if (isServlet22()) {
@@ -1207,8 +1209,9 @@
if (context == null) {
context = new ApplicationContext(getBasePath(), this);
- if (altDDName != null)
+ if (altDDName != null) {
context.setAttribute(Globals.ALT_DD_ATTR,altDDName);
+ }
}
return (context.getFacade());
@@ -1309,8 +1312,9 @@
}
DirContext oldResources = this.webappResources;
- if (oldResources == resources)
+ if (oldResources == resources) {
return;
+ }
if (resources instanceof BaseDirContext) {
((BaseDirContext) resources).setCached(isCachingAllowed());
@@ -1422,8 +1426,9 @@
synchronized (applicationListeners) {
String results[] =new String[applicationListeners.length +
1];
for (int i = 0; i < applicationListeners.length; i++) {
- if (listener.equals(applicationListeners[i]))
+ if (listener.equals(applicationListeners[i])) {
return;
+ }
results[i] = applicationListeners[i];
}
results[applicationListeners.length] = listener;
@@ -1473,9 +1478,10 @@
*/
public void addChild(Container child) {
- if (!(child instanceof Wrapper))
+ if (!(child instanceof Wrapper)) {
throw new IllegalArgumentException
(sm.getString("standardContext.notWrapper"));
+ }
Wrapper wrapper = (Wrapper) child;
String jspFile = wrapper.getJspFile();
if ((jspFile != null) && !jspFile.startsWith("/")) {
@@ -1504,11 +1510,12 @@
String patterns[] = collections[i].findPatterns();
for (int j = 0; j < patterns.length; j++) {
patterns[j] = adjustURLPattern(patterns[j]);
- if (!validateURLPattern(patterns[j]))
+ if (!validateURLPattern(patterns[j])) {
throw new IllegalArgumentException
(sm.getString
("standardContext.securityConstraint.pattern",
patterns[j]));
+ }
}
}
@@ -1516,8 +1523,9 @@
synchronized (constraints) {
SecurityConstraint results[] =
new SecurityConstraint[constraints.length + 1];
- for (int i = 0; i < constraints.length; i++)
+ for (int i = 0; i < constraints.length; i++) {
results[i] = constraints[i];
+ }
results[constraints.length] = constraint;
constraints = results;
}
@@ -1547,8 +1555,9 @@
public void addEnvironment(ContextEnvironment environment) {
ContextEnvironment env =
findEnvironment(environment.getName());
- if ((env != null) && !env.getOverride())
- return;
+ if ((env != null) && !env.getOverride()) {
+ return;
+ }
namingResources.addEnvironment(environment);
fireContainerEvent("addEnvironment", environment.getName());
@@ -1576,9 +1585,10 @@
public void addErrorPage(ErrorPage errorPage) {
// Validate the input parameters
- if (errorPage == null)
+ if (errorPage == null) {
throw new IllegalArgumentException
(sm.getString("standardContext.errorPage.required"));
+ }
String location = errorPage.getLocation();
if ((location != null) && !location.startsWith("/")) {
if (isServlet22()) {
@@ -1642,22 +1652,25 @@
String filterName = filterMap.getFilterName();
String servletName = filterMap.getServletName();
String urlPattern = filterMap.getURLPattern();
- if (findFilterDef(filterName) == null)
+ if (findFilterDef(filterName) == null) {
throw new IllegalArgumentException
(sm.getString("standardContext.filterMap.name",
filterName));
- if ((servletName == null) && (urlPattern == null))
+ }
+ if ((servletName == null) && (urlPattern == null)) {
throw new IllegalArgumentException
(sm.getString("standardContext.filterMap.either"));
- if ((servletName != null) && (urlPattern != null))
+ }
+ if ((servletName != null) && (urlPattern != null)) {
throw new IllegalArgumentException
(sm.getString("standardContext.filterMap.either"));
+ }
// Because filter-pattern is new in 2.3, no need to adjust
// for 2.2 backwards compatibility
- if ((urlPattern != null) && !validateURLPattern(urlPattern))
+ if ((urlPattern != null) && !validateURLPattern(urlPattern)) {
throw new IllegalArgumentException
(sm.getString("standardContext.filterMap.pattern",
urlPattern));
-
+ }
// Add this filter mapping to our registered set
synchronized (filterMaps) {
FilterMap results[] =new FilterMap[filterMaps.length + 1];
@@ -1679,9 +1692,10 @@
public void addInstanceListener(String listener) {
synchronized (instanceListeners) {
- String results[] =new String[instanceListeners.length + 1];
- for (int i = 0; i < instanceListeners.length; i++)
+ String results[] = new String[instanceListeners.length +
1];
+ for (int i = 0; i < instanceListeners.length; i++) {
results[i] = instanceListeners[i];
+ }
results[instanceListeners.length] = listener;
instanceListeners = results;
}
@@ -1705,7 +1719,7 @@
}
// Properly handle file that are considered to be a jsp.
- if (pattern.indexOf("*.") > 0){
+ if (pattern.indexOf("*.") > 0) {
pattern = pattern.substring(pattern.lastIndexOf("*"));
servletName = "jsp";
}
@@ -1800,13 +1814,14 @@
public void addParameter(String name, String value) {
// Validate the proposed context initialization parameter
- if ((name == null) || (value == null))
+ if ((name == null) || (value == null)) {
throw new IllegalArgumentException
(sm.getString("standardContext.parameter.required"));
- if (parameters.get(name) != null)
+ }
+ if (parameters.get(name) != null) {
throw new IllegalArgumentException
(sm.getString("standardContext.parameter.duplicate",
name));
-
+ }
// Add this parameter to our defined set
synchronized (parameters) {
parameters.put(name, value);
@@ -1880,9 +1895,10 @@
public void addSecurityRole(String role) {
synchronized (securityRoles) {
- String results[] =new String[securityRoles.length + 1];
- for (int i = 0; i < securityRoles.length; i++)
+ String results[] = new String[securityRoles.length + 1];
+ for (int i = 0; i < securityRoles.length; i++) {
results[i] = securityRoles[i];
+ }
results[securityRoles.length] = role;
securityRoles = results;
}
@@ -1904,13 +1920,15 @@
public void addServletMapping(String pattern, String name) {
// Validate the proposed mapping
- if (findChild(name) == null)
+ if (findChild(name) == null) {
throw new IllegalArgumentException
(sm.getString("standardContext.servletMap.name",
name));
+ }
pattern = adjustURLPattern(RequestUtil.URLDecode(pattern));
- if (!validateURLPattern(pattern))
+ if (!validateURLPattern(pattern)) {
throw new IllegalArgumentException
- (sm.getString("standardContext.servletMap.pattern",
pattern));
+ (sm.getString("standardContext.servletMap.pattern",
pattern));
+ }
// Add this mapping to our registered set
synchronized (servletMappings) {
@@ -1954,8 +1972,9 @@
setReplaceWelcomeFiles(false);
}
String results[] =new String[welcomeFiles.length + 1];
- for (int i = 0; i < welcomeFiles.length; i++)
+ for (int i = 0; i < welcomeFiles.length; i++) {
results[i] = welcomeFiles[i];
+ }
results[welcomeFiles.length] = name;
welcomeFiles = results;
}
@@ -1975,8 +1994,9 @@
synchronized (wrapperLifecycles) {
String results[] =new String[wrapperLifecycles.length + 1];
- for (int i = 0; i < wrapperLifecycles.length; i++)
+ for (int i = 0; i < wrapperLifecycles.length; i++) {
results[i] = wrapperLifecycles[i];
+ }
results[wrapperLifecycles.length] = listener;
wrapperLifecycles = results;
}
@@ -1995,8 +2015,9 @@
synchronized (wrapperListeners) {
String results[] =new String[wrapperListeners.length + 1];
- for (int i = 0; i < wrapperListeners.length; i++)
+ for (int i = 0; i < wrapperListeners.length; i++) {
results[i] = wrapperListeners[i];
+ }
results[wrapperListeners.length] = listener;
wrapperListeners = results;
}
@@ -2036,8 +2057,9 @@
Class clazz = Class.forName(wrapperLifecycles[i]);
LifecycleListener listener =
(LifecycleListener) clazz.newInstance();
- if (wrapper instanceof Lifecycle)
+ if (wrapper instanceof Lifecycle) {
((Lifecycle)
wrapper).addLifecycleListener(listener);
+ }
} catch (Throwable t) {
log.error("createWrapper", t);
return (null);
@@ -2193,10 +2215,12 @@
(ErrorPage[])
statusPages.values().toArray(results2);
ErrorPage results[] =
new ErrorPage[results1.length + results2.length];
- for (int i = 0; i < results1.length; i++)
+ for (int i = 0; i < results1.length; i++) {
results[i] = results1[i];
- for (int i = results1.length; i < results.length; i++)
+ }
+ for (int i = results1.length; i < results.length; i++)
{
results[i] = results2[i - results1.length];
+ }
return (results);
}
}
@@ -2480,10 +2504,11 @@
synchronized (roleMappings) {
realRole = (String) roleMappings.get(role);
}
- if (realRole != null)
+ if (realRole != null) {
return (realRole);
- else
+ } else {
return (role);
+ }
}
@@ -2498,8 +2523,9 @@
synchronized (securityRoles) {
for (int i = 0; i < securityRoles.length; i++) {
- if (role.equals(securityRoles[i]))
+ if (role.equals(securityRoles[i])) {
return (true);
+ }
}
}
return (false);
@@ -2572,8 +2598,9 @@
int results[] = new int[statusPages.size()];
Iterator elements = statusPages.keySet().iterator();
int i = 0;
- while (elements.hasNext())
+ while (elements.hasNext()) {
results[i++] = ((Integer) elements.next()).intValue();
+ }
return (results);
}
@@ -2618,13 +2645,10 @@
*/
public boolean findWelcomeFile(String name) {
- synchronized (welcomeFiles) {
- for (int i = 0; i < welcomeFiles.length; i++) {
- if (name.equals(welcomeFiles[i]))
- return (true);
- }
- }
- return (false);
+ synchronized (welcomeFiles) {
+ ArrayList files = (ArrayList)Arrays.asList(welcomeFiles);
+ return (files.contains(name)) ;
+ }
}
@@ -2952,8 +2976,9 @@
ApplicationParameter results[] =
new ApplicationParameter[applicationParameters.length
- 1];
for (int i = 0; i < applicationParameters.length; i++) {
- if (i != n)
+ if (i != n) {
results[j++] = applicationParameters[i];
+ }
}
applicationParameters = results;
@@ -4877,8 +4902,8 @@
Registry.getRegistry().registerComponent(this,oname, null);
}
}
- for( Iterator it=wrappers.iterator(); it.hasNext() ; ) {
- StandardWrapper wrapper=(StandardWrapper)it.next();
+ for(int i=0, size=wrappers.size(); i<size; i++) {
+ StandardWrapper wrapper =
(StandardWrapper)wrappers.get(i);
// XXX prevent duplicated registration
wrapper.registerJMX( this );
}
--
"To be governed is to be watched over, inspected, spied on, directed,
legislated..." - Pierre-Joseph Proudhon
---------------------------------------------------------------------
To unsubscribe, e-mail: tomcat-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: tomcat-dev-help@jakarta.apache.org
Re: [patch]StandardContext general code cleanup and findWelcomeFiles
alternate implementation
Posted by Remy Maucherat <re...@apache.org>.
kev wrote:
> I've gone through most of the code in StandardContext and cleaned up
> if/for {} to try and keep the code consistent across the class. I've
> also changed findWelcomeFiles to remove the two returns from the method.
Ok, why not. Few people clean up the code, so I suppose it's useful.
In the case of Tomcat 4.1.x, I'd like to only apply bugfix related
patches (ie, no beautifying, or no feature additions at this point).
Remy
---------------------------------------------------------------------
To unsubscribe, e-mail: tomcat-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: tomcat-dev-help@jakarta.apache.org
Re: [5.0] Xerces 2 impact on startup
Posted by Jean-Francois Arcand <jf...@apache.org>.
Remy Maucherat wrote:
> Just an observation ...
> When I remove Xerces 2.3 and rely on the Crimson parser integrated in
> my new JDK (Sun 1.4.2 beta), startup time goes down 20%. Impressive
> difference (I would have thought the later 2.x releases would improve
> performance :-( ).
And it is not better with 2.4 :-(. As Costin often recommend, we should
start thinking having a "deploy tool/validation tool" where Xerces will
be used only if the web-app uses schema for validation. IMO, we can keep
the validation part as it is but improve the error message in case
validation is turned on, schema is used and Crimson throws an exception.
Redirect people to the "validation tool". Something like:
catalina validate -webapp= .
This script can simply call xerces with validation turned on. And then
we can start improving the error message (the schema error produced
currently are ugly) by applying xslt to the output.
And only the deploy tool will be broken when a new Xerces is out (just
kidding ;-)).....
-- Jeanfrancois
>
>
> Remy
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: tomcat-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: tomcat-dev-help@jakarta.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: tomcat-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: tomcat-dev-help@jakarta.apache.org
Re: [5.0] Xerces 2 impact on startup
Posted by Costin Manolache <cm...@yahoo.com>.
Remy Maucherat wrote:
> Costin Manolache wrote:
>> Remy Maucherat wrote:
>>
>> I agree - the startup time is important. Tomcat itself ( with no app )
>> starts very quickly - and IMO caching the context info ( in a .ser file )
>> would take care of most of it. That's why I started to add "Serialiable"
>> markers and sort out the "transient" fields that shouldn't be saved.
>>
>> However there are many more important issues - I don't expect this to get
>> into 5.0. Getting the code stable and finishing few remaining open issues
>> should be the biggest priority.
>>
>> Moving back to crimson - or no parser at all for JDK1.4+ - and having
>> a small bundle with ant+xerces+a script that validates an XML file
>> against DTD and schema would be an improvement for startup time,
>> and reduce the size and complexity. My +1 if anyone wants to propose it
>> :-)
>
> Yes, that sounds reasonable.
> We could pitch the main distribution as JDK 1.4+, and provide a package
> of binaries for older VMs, as well as that deployer package ?
I have no problem with having the main distribution targeted to JDK1.4,
or recommending 1.4.
This has been discussed many times - what's important is to still be able to
run tomcat with JDK1.3 and GCJ, for people who can't use JDK1.4.
GCJ is important for other reasons as well.
Costin
> Obviously, this may not happen unless contributors show up (this could
> be a nice small project for new developers, hint, hint) :)
---------------------------------------------------------------------
To unsubscribe, e-mail: tomcat-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: tomcat-dev-help@jakarta.apache.org
Re: [5.0] Xerces 2 impact on startup
Posted by Remy Maucherat <re...@apache.org>.
Costin Manolache wrote:
> Remy Maucherat wrote:
>
> I agree - the startup time is important. Tomcat itself ( with no app )
> starts very quickly - and IMO caching the context info ( in a .ser file )
> would take care of most of it. That's why I started to add "Serialiable"
> markers and sort out the "transient" fields that shouldn't be saved.
>
> However there are many more important issues - I don't expect this to get
> into 5.0. Getting the code stable and finishing few remaining open issues
> should be the biggest priority.
>
> Moving back to crimson - or no parser at all for JDK1.4+ - and having
> a small bundle with ant+xerces+a script that validates an XML file
> against DTD and schema would be an improvement for startup time,
> and reduce the size and complexity. My +1 if anyone wants to propose it :-)
Yes, that sounds reasonable.
We could pitch the main distribution as JDK 1.4+, and provide a package
of binaries for older VMs, as well as that deployer package ?
Obviously, this may not happen unless contributors show up (this could
be a nice small project for new developers, hint, hint) :)
Remy
---------------------------------------------------------------------
To unsubscribe, e-mail: tomcat-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: tomcat-dev-help@jakarta.apache.org
Re: [5.0] Xerces 2 impact on startup
Posted by Costin Manolache <cm...@yahoo.com>.
Remy Maucherat wrote:
> Costin Manolache wrote:
>> Remy Maucherat wrote:
>>
>>
>>>Just an observation ...
>>>When I remove Xerces 2.3 and rely on the Crimson parser integrated in my
>>>new JDK (Sun 1.4.2 beta), startup time goes down 20%. Impressive
>>>difference (I would have thought the later 2.x releases would improve
>>>performance :-( ).
>>
>>
>> Well, you know my position - Xerces is a great parser, but we should
>> distribute crimson ( for JDK1.3 ) and use the parser include in 1.4
>> otherwise.
>>
>> It's not only speed - size also matters, plus the idea that we only need
>> SAX for tomcat runtime.
>
> Indeed, it's only 200k.
>
> <sarcasm>
> Anyone around to point out it can't do schema validation ? (which as we
> all know, is a killer feature; for performance, that is ;-) )
> </sarcasm>
>
> About the startup time, I didn't try to profile or optimize it. I'll
> continue focusing on other optimizations for now, but have to admit this
> is very important for good user experience (like JSP compilation for
> webapps).
I agree - the startup time is important. Tomcat itself ( with no app )
starts very quickly - and IMO caching the context info ( in a .ser file )
would take care of most of it. That's why I started to add "Serialiable"
markers and sort out the "transient" fields that shouldn't be saved.
However there are many more important issues - I don't expect this to get
into 5.0. Getting the code stable and finishing few remaining open issues
should be the biggest priority.
Moving back to crimson - or no parser at all for JDK1.4+ - and having
a small bundle with ant+xerces+a script that validates an XML file
against DTD and schema would be an improvement for startup time,
and reduce the size and complexity. My +1 if anyone wants to propose it :-)
Costin
---------------------------------------------------------------------
To unsubscribe, e-mail: tomcat-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: tomcat-dev-help@jakarta.apache.org
Re: [5.0] Xerces 2 impact on startup
Posted by Remy Maucherat <re...@apache.org>.
Costin Manolache wrote:
> Remy Maucherat wrote:
>
>
>>Just an observation ...
>>When I remove Xerces 2.3 and rely on the Crimson parser integrated in my
>>new JDK (Sun 1.4.2 beta), startup time goes down 20%. Impressive
>>difference (I would have thought the later 2.x releases would improve
>>performance :-( ).
>
>
> Well, you know my position - Xerces is a great parser, but we should
> distribute crimson ( for JDK1.3 ) and use the parser include in 1.4
> otherwise.
>
> It's not only speed - size also matters, plus the idea that we only need SAX
> for tomcat runtime.
Indeed, it's only 200k.
<sarcasm>
Anyone around to point out it can't do schema validation ? (which as we
all know, is a killer feature; for performance, that is ;-) )
</sarcasm>
About the startup time, I didn't try to profile or optimize it. I'll
continue focusing on other optimizations for now, but have to admit this
is very important for good user experience (like JSP compilation for
webapps).
Remy
---------------------------------------------------------------------
To unsubscribe, e-mail: tomcat-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: tomcat-dev-help@jakarta.apache.org
Re: [5.0] Xerces 2 impact on startup
Posted by Costin Manolache <cm...@yahoo.com>.
Remy Maucherat wrote:
> Just an observation ...
> When I remove Xerces 2.3 and rely on the Crimson parser integrated in my
> new JDK (Sun 1.4.2 beta), startup time goes down 20%. Impressive
> difference (I would have thought the later 2.x releases would improve
> performance :-( ).
Well, you know my position - Xerces is a great parser, but we should
distribute crimson ( for JDK1.3 ) and use the parser include in 1.4
otherwise.
It's not only speed - size also matters, plus the idea that we only need SAX
for tomcat runtime.
Costin
---------------------------------------------------------------------
To unsubscribe, e-mail: tomcat-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: tomcat-dev-help@jakarta.apache.org
Re: [5.0] Xerces 2 impact on startup
Posted by Remy Maucherat <re...@apache.org>.
Jess M. Holle wrote:
> Given that Xerces is a heavier (byte-code-quantity-wise) implementation,
> I would guess that initial classload of this would take substantially
> longer than Crimson for starters.
>
> The more interesting/worrisome bit is if additional operations
> thereafter are much slower...
Well, TC startup is rather segmented (first, modeler descriptors, then
server.xml parsing, then everything else). I added numbers for the 3
operations, and all three are consistently 20% slower. Not that it
wasn't news for Xerces 2.0.x; I thought it would then improve (like TC
4.x performance, which improves gradually), silly me :)
Remy
---------------------------------------------------------------------
To unsubscribe, e-mail: tomcat-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: tomcat-dev-help@jakarta.apache.org
Re: [5.0] Xerces 2 impact on startup
Posted by "Jess M. Holle" <je...@ptc.com>.
Given that Xerces is a heavier (byte-code-quantity-wise) implementation,
I would guess that initial classload of this would take substantially
longer than Crimson for starters.
The more interesting/worrisome bit is if additional operations
thereafter are much slower...
Remy Maucherat wrote:
> Just an observation ...
> When I remove Xerces 2.3 and rely on the Crimson parser integrated in
> my new JDK (Sun 1.4.2 beta), startup time goes down 20%. Impressive
> difference (I would have thought the later 2.x releases would improve
> performance :-( ).
>
> Remy
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: tomcat-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: tomcat-dev-help@jakarta.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: tomcat-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: tomcat-dev-help@jakarta.apache.org
[5.0] Xerces 2 impact on startup
Posted by Remy Maucherat <re...@apache.org>.
Just an observation ...
When I remove Xerces 2.3 and rely on the Crimson parser integrated in my
new JDK (Sun 1.4.2 beta), startup time goes down 20%. Impressive
difference (I would have thought the later 2.x releases would improve
performance :-( ).
Remy
---------------------------------------------------------------------
To unsubscribe, e-mail: tomcat-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: tomcat-dev-help@jakarta.apache.org