You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by GitBox <gi...@apache.org> on 2020/09/04 16:40:49 UTC
[GitHub] [tomcat] kamnani opened a new pull request #354: Optimize Server startup time using multi-threading for annotation scanning
kamnani opened a new pull request #354:
URL: https://github.com/apache/tomcat/pull/354
This is a redo of Previous PR: https://github.com/apache/tomcat/pull/349
The following changes have been made based on the suggestions earlier:
1) Flags (parallelAnnotationScanning & maxThreadsForParallelAnnotationScanning) can be passed through Host Configuration. See example below.
2) By default parallelAnnotationScanning remains disabled and thus default tomcat behavior is untouched.
3) maxThreadsForParallelAnnotationScanning defaults to 1 and will be in-effect if parallelAnnotationScanning is enabled via Host Configs.
3) PR is against master.
Apologies in case something is missed.
```
<Host name="localhost" appBase="webapps"
unpackWARs="true" autoDeploy="true" parallelAnnotationScanning="false" maxThreadsForParallelAnnotationScanning="10">
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] kamnani commented on a change in pull request #354: Optimize Server startup time using multi-threading for annotation scanning
Posted by GitBox <gi...@apache.org>.
kamnani commented on a change in pull request #354:
URL: https://github.com/apache/tomcat/pull/354#discussion_r485073735
##########
File path: java/org/apache/catalina/startup/ContextConfig.java
##########
@@ -1374,7 +1383,19 @@ protected void webConfig() {
protected void processClasses(WebXml webXml, Set<WebXml> orderedFragments) {
// Step 4. Process /WEB-INF/classes for annotations and
// @HandlesTypes matches
- Map<String, JavaClassCacheEntry> javaClassCache = new HashMap<>();
+
+ Map<String, JavaClassCacheEntry> javaClassCache;
+
+ if (context.getParent() instanceof Host) {
+ Host host = (Host) context.getParent();
+ Container container = host.getParent();
+ CONCURRENCY_LEVEL = container.getStartStopThreads();
+ javaClassCache = new ConcurrentHashMap<>(DEFAULT_CLASS_CACHE_SIZE, DEFAULT_LOAD_FACTOR,
+ CONCURRENCY_LEVEL);
+ } else {
+ javaClassCache = new ConcurrentHashMap<>(DEFAULT_CLASS_CACHE_SIZE, DEFAULT_LOAD_FACTOR,
+ CONCURRENCY_LEVEL);
+ }
Review comment:
Have added the default behavior without the flag in the latest commit.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] rmaucher commented on pull request #354: Optimize Server startup time using multi-threading for annotation scanning
Posted by GitBox <gi...@apache.org>.
rmaucher commented on pull request #354:
URL: https://github.com/apache/tomcat/pull/354#issuecomment-687873084
I still don't really like some items, like the explicit flag (but I don't see yet how to do it) or some of the "optimizations".
By default the utility thread count is 2, so not too much parallelism.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] rmaucher commented on a change in pull request #354: Optimize Server startup time using multi-threading for annotation scanning
Posted by GitBox <gi...@apache.org>.
rmaucher commented on a change in pull request #354:
URL: https://github.com/apache/tomcat/pull/354#discussion_r484426457
##########
File path: java/org/apache/catalina/startup/ContextConfig.java
##########
@@ -122,6 +126,11 @@
private static final Log log = LogFactory.getLog(ContextConfig.class);
+ private static final int DEFAULT_CLASS_CACHE_SIZE = 16384;
+
+ private static final float DEFAULT_LOAD_FACTOR = .75f;
+
+ private static int CONCURRENCY_LEVEL = 1;
Review comment:
Ok for the map size, maybe (it seems very large, 16k classes ?), I don't quite see the point of the rest. The concurrency may be up to the utility executor thread count, and I don't think this is an optimization which makes any difference.
##########
File path: java/org/apache/catalina/startup/ContextConfig.java
##########
@@ -1374,7 +1383,19 @@ protected void webConfig() {
protected void processClasses(WebXml webXml, Set<WebXml> orderedFragments) {
// Step 4. Process /WEB-INF/classes for annotations and
// @HandlesTypes matches
- Map<String, JavaClassCacheEntry> javaClassCache = new HashMap<>();
+
+ Map<String, JavaClassCacheEntry> javaClassCache;
+
+ if (context.getParent() instanceof Host) {
+ Host host = (Host) context.getParent();
+ Container container = host.getParent();
+ CONCURRENCY_LEVEL = container.getStartStopThreads();
+ javaClassCache = new ConcurrentHashMap<>(DEFAULT_CLASS_CACHE_SIZE, DEFAULT_LOAD_FACTOR,
+ CONCURRENCY_LEVEL);
+ } else {
+ javaClassCache = new ConcurrentHashMap<>(DEFAULT_CLASS_CACHE_SIZE, DEFAULT_LOAD_FACTOR,
+ CONCURRENCY_LEVEL);
+ }
Review comment:
Well, if it's changed to be always a ConcurrentHashMap, it probably won't be a measurable performance difference.
##########
File path: java/org/apache/catalina/startup/ContextConfig.java
##########
@@ -2136,26 +2157,98 @@ protected InputSource getWebXmlSource(String filename, boolean global) {
}
protected void processAnnotations(Set<WebXml> fragments,
- boolean handlesTypesOnly, Map<String,JavaClassCacheEntry> javaClassCache) {
- for(WebXml fragment : fragments) {
- // Only need to scan for @HandlesTypes matches if any of the
- // following are true:
- // - it has already been determined only @HandlesTypes is required
- // (e.g. main web.xml has metadata-complete="true"
- // - this fragment is for a container JAR (Servlet 3.1 section 8.1)
- // - this fragment has metadata-complete="true"
- boolean htOnly = handlesTypesOnly || !fragment.getWebappJar() ||
- fragment.isMetadataComplete();
-
- WebXml annotations = new WebXml();
- // no impact on distributable
- annotations.setDistributable(true);
- URL url = fragment.getURL();
- processAnnotationsUrl(url, annotations, htOnly, javaClassCache);
- Set<WebXml> set = new HashSet<>();
- set.add(annotations);
- // Merge annotations into fragment - fragment takes priority
- fragment.merge(set);
+ boolean handlesTypesOnly, Map<String, JavaClassCacheEntry> javaClassCache) {
+
+ if (context.getParent() instanceof Host && ((Host) context.getParent()).isParallelAnnotationScanning()) {
+ processAnnotationsInParallel(fragments, handlesTypesOnly, javaClassCache);
+ return;
+ }
+
+ for (WebXml fragment : fragments) {
+ scanWebXmlFragment(handlesTypesOnly, fragment, javaClassCache);
+ }
+ }
+
+ private void scanWebXmlFragment(boolean handlesTypesOnly, WebXml fragment, Map<String, JavaClassCacheEntry> javaClassCache) {
+
+ // Only need to scan for @HandlesTypes matches if any of the
+ // following are true:
+ // - it has already been determined only @HandlesTypes is required
+ // (e.g. main web.xml has metadata-complete="true"
+ // - this fragment is for a container JAR (Servlet 3.1 section 8.1)
+ // - this fragment has metadata-complete="true"
+ boolean htOnly = handlesTypesOnly || !fragment.getWebappJar() ||
+ fragment.isMetadataComplete();
+
+ WebXml annotations = new WebXml();
+ // no impact on distributable
+ annotations.setDistributable(true);
+ URL url = fragment.getURL();
+ processAnnotationsUrl(url, annotations, htOnly, javaClassCache);
+ Set<WebXml> set = new HashSet<>();
+ set.add(annotations);
+ // Merge annotations into fragment - fragment takes priority
+ fragment.merge(set);
+ }
+
+ /**
+ * Executable task to scan a segment for annotations. Each task does the
+ * same work as the for loop inside processAnnotations();
+ *
+ * @author Engebretson, John
+ * @author Kamnani, Jatin
+ */
+ private class AnnotationScanTask implements Callable<Void> {
+ private final WebXml fragment;
+ private final boolean handlesTypesOnly;
+ private Map<String, JavaClassCacheEntry> javaClassCache;
+
+ private AnnotationScanTask(WebXml fragment, boolean handlesTypesOnly, Map<String, JavaClassCacheEntry> javaClassCache) {
+ this.fragment = fragment;
+ this.handlesTypesOnly = handlesTypesOnly;
+ this.javaClassCache = javaClassCache;
+ }
+
+ @Override
+ public Void call() {
+ scanWebXmlFragment(handlesTypesOnly, fragment, javaClassCache);
+
+ return null;
+ }
+
+ }
+
+ /**
+ * Parallelized version of processAnnotationsInParallel(). Constructs tasks,
+ * submits them as they're created, then waits for completion.
+ *
+ * @param fragments Set of parallelizable scans
+ * @param handlesTypesOnly Important parameter for the underlying scan
+ */
+ protected void processAnnotationsInParallel(Set<WebXml> fragments, boolean handlesTypesOnly,
+ Map<String, JavaClassCacheEntry> javaClassCache) {
+
+
+ Server s = getServer();
+ ExecutorService pool = null;
+ try {
+ pool = s.getUtilityExecutor();
+ List<Future<Void>> futures = new ArrayList<>(fragments.size());
+ for (WebXml fragment : fragments) {
+ Callable<Void> task = new AnnotationScanTask(fragment, handlesTypesOnly, javaClassCache);
+ futures.add(pool.submit(task));
+ }
+ try {
+ for (Future<?> future : futures) {
+ future.get();
+ }
+ } catch (Exception e) {
+ throw new RuntimeException("Parallel execution failed", e);
Review comment:
Localization missing.
##########
File path: java/org/apache/catalina/mbeans/MBeanFactory.java
##########
@@ -490,6 +491,7 @@ public String createStandardContext(String parent,
public String createStandardHost(String parent, String name,
String appBase,
boolean autoDeploy,
+ boolean parallelAnnotationScanning,
Review comment:
I disagree with this API change. IMO this new feature does not justify it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] kamnani edited a comment on pull request #354: Optimize Server startup time using multi-threading for annotation scanning
Posted by GitBox <gi...@apache.org>.
kamnani edited a comment on pull request #354:
URL: https://github.com/apache/tomcat/pull/354#issuecomment-688967898
> I still don't really like some items, like the explicit flag (but I don't see yet how to do it) or some of the "optimizations".
> By default the utility thread count is 2, so not too much parallelism.
Yes, You're right with the default value, but that can be changed through the Host Configuration.. right?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] kamnani commented on a change in pull request #354: Optimize Server startup time using multi-threading for annotation scanning
Posted by GitBox <gi...@apache.org>.
kamnani commented on a change in pull request #354:
URL: https://github.com/apache/tomcat/pull/354#discussion_r485072258
##########
File path: java/org/apache/catalina/startup/ContextConfig.java
##########
@@ -122,6 +126,11 @@
private static final Log log = LogFactory.getLog(ContextConfig.class);
+ private static final int DEFAULT_CLASS_CACHE_SIZE = 16384;
+
+ private static final float DEFAULT_LOAD_FACTOR = .75f;
+
+ private static int CONCURRENCY_LEVEL = 1;
Review comment:
Resolved with latest commit.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] rmaucher commented on a change in pull request #354: Optimize Server startup time using multi-threading for annotation scanning
Posted by GitBox <gi...@apache.org>.
rmaucher commented on a change in pull request #354:
URL: https://github.com/apache/tomcat/pull/354#discussion_r486924037
##########
File path: java/org/apache/catalina/startup/LocalStrings_fr.properties
##########
@@ -69,6 +69,7 @@ contextConfig.missingRealm=Aucun royaume (realm) n'a été configuré pour réal
contextConfig.processAnnotationsDir.debug=Balayage du répertoire pour trouver des fichiers de classe avec annotations [{0}]
contextConfig.processAnnotationsJar.debug=Analyse du fichier jars pour des classes annotées avec [{0}]
contextConfig.processAnnotationsWebDir.debug=Balayage du répertoire d''applications web, pour fichiers de classe avec annotations [{0}]
+contextConfig.processAnnotationsInParallelFailure=exécution parallèle a échoué
Review comment:
Yes, the translation is meh. As a rule, I'd say never commit any localization IMO, just let others handle in with poeditor later.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] kamnani commented on a change in pull request #354: Optimize Server startup time using multi-threading for annotation scanning
Posted by GitBox <gi...@apache.org>.
kamnani commented on a change in pull request #354:
URL: https://github.com/apache/tomcat/pull/354#discussion_r485091168
##########
File path: java/org/apache/catalina/mbeans/MBeanFactory.java
##########
@@ -490,6 +491,7 @@ public String createStandardContext(String parent,
public String createStandardHost(String parent, String name,
String appBase,
boolean autoDeploy,
+ boolean parallelAnnotationScanning,
Review comment:
Thanks @rmaucher for the comment.
I have little experience working on Tomcat so if you can you explain how this becomes irrelevant that would be great. Also, do we not want the API's to be able to pass this flag when the optimizations are quite significant?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] kamnani commented on pull request #354: Optimize Server startup time using multi-threading for annotation scanning
Posted by GitBox <gi...@apache.org>.
kamnani commented on pull request #354:
URL: https://github.com/apache/tomcat/pull/354#issuecomment-687380392
Thanks @rmaucher for your comment. I have made the necessary changes as suggested. It would be great if you could verify that change. I have also updated the PR description since the flags have now been changed with this change.
Is there any other change needed on this PR you want to suggest ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] kamnani commented on a change in pull request #354: Optimize Server startup time using multi-threading for annotation scanning
Posted by GitBox <gi...@apache.org>.
kamnani commented on a change in pull request #354:
URL: https://github.com/apache/tomcat/pull/354#discussion_r489070249
##########
File path: java/org/apache/catalina/startup/ContextConfig.java
##########
@@ -2136,26 +2146,98 @@ protected InputSource getWebXmlSource(String filename, boolean global) {
}
protected void processAnnotations(Set<WebXml> fragments,
- boolean handlesTypesOnly, Map<String,JavaClassCacheEntry> javaClassCache) {
- for(WebXml fragment : fragments) {
- // Only need to scan for @HandlesTypes matches if any of the
- // following are true:
- // - it has already been determined only @HandlesTypes is required
- // (e.g. main web.xml has metadata-complete="true"
- // - this fragment is for a container JAR (Servlet 3.1 section 8.1)
- // - this fragment has metadata-complete="true"
- boolean htOnly = handlesTypesOnly || !fragment.getWebappJar() ||
- fragment.isMetadataComplete();
-
- WebXml annotations = new WebXml();
- // no impact on distributable
- annotations.setDistributable(true);
- URL url = fragment.getURL();
- processAnnotationsUrl(url, annotations, htOnly, javaClassCache);
- Set<WebXml> set = new HashSet<>();
- set.add(annotations);
- // Merge annotations into fragment - fragment takes priority
- fragment.merge(set);
+ boolean handlesTypesOnly, Map<String, JavaClassCacheEntry> javaClassCache) {
+
+ if (context.getParent() instanceof Host && ((Host) context.getParent()).isParallelAnnotationScanning()) {
+ processAnnotationsInParallel(fragments, handlesTypesOnly, javaClassCache);
+ return;
+ }
+
+ for (WebXml fragment : fragments) {
+ scanWebXmlFragment(handlesTypesOnly, fragment, javaClassCache);
+ }
+ }
+
+ private void scanWebXmlFragment(boolean handlesTypesOnly, WebXml fragment, Map<String, JavaClassCacheEntry> javaClassCache) {
+
+ // Only need to scan for @HandlesTypes matches if any of the
+ // following are true:
+ // - it has already been determined only @HandlesTypes is required
+ // (e.g. main web.xml has metadata-complete="true"
+ // - this fragment is for a container JAR (Servlet 3.1 section 8.1)
+ // - this fragment has metadata-complete="true"
+ boolean htOnly = handlesTypesOnly || !fragment.getWebappJar() ||
+ fragment.isMetadataComplete();
+
+ WebXml annotations = new WebXml();
+ // no impact on distributable
+ annotations.setDistributable(true);
+ URL url = fragment.getURL();
+ processAnnotationsUrl(url, annotations, htOnly, javaClassCache);
+ Set<WebXml> set = new HashSet<>();
+ set.add(annotations);
+ // Merge annotations into fragment - fragment takes priority
+ fragment.merge(set);
+ }
+
+ /**
+ * Executable task to scan a segment for annotations. Each task does the
+ * same work as the for loop inside processAnnotations();
+ *
+ * @author Engebretson, John
+ * @author Kamnani, Jatin
+ */
+ private class AnnotationScanTask implements Callable<Void> {
Review comment:
Thanks for the suggestion @martin-g .
Pushed the changes as per the latest feedback. Can you have a look on this PR again ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] martin-g commented on a change in pull request #354: Optimize Server startup time using multi-threading for annotation scanning
Posted by GitBox <gi...@apache.org>.
martin-g commented on a change in pull request #354:
URL: https://github.com/apache/tomcat/pull/354#discussion_r486872606
##########
File path: java/org/apache/catalina/startup/ContextConfig.java
##########
@@ -2136,26 +2146,98 @@ protected InputSource getWebXmlSource(String filename, boolean global) {
}
protected void processAnnotations(Set<WebXml> fragments,
- boolean handlesTypesOnly, Map<String,JavaClassCacheEntry> javaClassCache) {
- for(WebXml fragment : fragments) {
- // Only need to scan for @HandlesTypes matches if any of the
- // following are true:
- // - it has already been determined only @HandlesTypes is required
- // (e.g. main web.xml has metadata-complete="true"
- // - this fragment is for a container JAR (Servlet 3.1 section 8.1)
- // - this fragment has metadata-complete="true"
- boolean htOnly = handlesTypesOnly || !fragment.getWebappJar() ||
- fragment.isMetadataComplete();
-
- WebXml annotations = new WebXml();
- // no impact on distributable
- annotations.setDistributable(true);
- URL url = fragment.getURL();
- processAnnotationsUrl(url, annotations, htOnly, javaClassCache);
- Set<WebXml> set = new HashSet<>();
- set.add(annotations);
- // Merge annotations into fragment - fragment takes priority
- fragment.merge(set);
+ boolean handlesTypesOnly, Map<String, JavaClassCacheEntry> javaClassCache) {
+
+ if (context.getParent() instanceof Host && ((Host) context.getParent()).isParallelAnnotationScanning()) {
+ processAnnotationsInParallel(fragments, handlesTypesOnly, javaClassCache);
+ return;
+ }
+
+ for (WebXml fragment : fragments) {
+ scanWebXmlFragment(handlesTypesOnly, fragment, javaClassCache);
+ }
+ }
+
+ private void scanWebXmlFragment(boolean handlesTypesOnly, WebXml fragment, Map<String, JavaClassCacheEntry> javaClassCache) {
+
+ // Only need to scan for @HandlesTypes matches if any of the
+ // following are true:
+ // - it has already been determined only @HandlesTypes is required
+ // (e.g. main web.xml has metadata-complete="true"
+ // - this fragment is for a container JAR (Servlet 3.1 section 8.1)
+ // - this fragment has metadata-complete="true"
+ boolean htOnly = handlesTypesOnly || !fragment.getWebappJar() ||
+ fragment.isMetadataComplete();
+
+ WebXml annotations = new WebXml();
+ // no impact on distributable
+ annotations.setDistributable(true);
+ URL url = fragment.getURL();
+ processAnnotationsUrl(url, annotations, htOnly, javaClassCache);
+ Set<WebXml> set = new HashSet<>();
+ set.add(annotations);
+ // Merge annotations into fragment - fragment takes priority
+ fragment.merge(set);
+ }
+
+ /**
+ * Executable task to scan a segment for annotations. Each task does the
+ * same work as the for loop inside processAnnotations();
+ *
+ * @author Engebretson, John
+ * @author Kamnani, Jatin
+ */
+ private class AnnotationScanTask implements Callable<Void> {
+ private final WebXml fragment;
+ private final boolean handlesTypesOnly;
+ private Map<String, JavaClassCacheEntry> javaClassCache;
+
+ private AnnotationScanTask(WebXml fragment, boolean handlesTypesOnly, Map<String, JavaClassCacheEntry> javaClassCache) {
+ this.fragment = fragment;
+ this.handlesTypesOnly = handlesTypesOnly;
+ this.javaClassCache = javaClassCache;
+ }
+
+ @Override
+ public Void call() {
+ scanWebXmlFragment(handlesTypesOnly, fragment, javaClassCache);
+
+ return null;
+ }
+
+ }
+
+ /**
+ * Parallelized version of processAnnotationsInParallel(). Constructs tasks,
+ * submits them as they're created, then waits for completion.
+ *
+ * @param fragments Set of parallelizable scans
+ * @param handlesTypesOnly Important parameter for the underlying scan
+ */
+ protected void processAnnotationsInParallel(Set<WebXml> fragments, boolean handlesTypesOnly,
+ Map<String, JavaClassCacheEntry> javaClassCache) {
+
+
+ Server s = getServer();
+ ExecutorService pool = null;
+ try {
+ pool = s.getUtilityExecutor();
+ List<Future<Void>> futures = new ArrayList<>(fragments.size());
+ for (WebXml fragment : fragments) {
+ Callable<Void> task = new AnnotationScanTask(fragment, handlesTypesOnly, javaClassCache);
+ futures.add(pool.submit(task));
+ }
+ try {
+ for (Future<?> future : futures) {
+ future.get();
+ }
+ } catch (Exception e) {
+ throw new RuntimeException(sm.getString("contextConfig.processAnnotationsInParallelFailure"), e);
+ }
+ } finally {
+ if (pool != null) {
+ pool.shutdownNow();
Review comment:
This is not really needed because org.apache.tomcat.util.threads.ScheduledThreadPoolExecutor#shutdown methods do nothing, but anyway it is a good practice to do it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] martin-g commented on a change in pull request #354: Optimize Server startup time using multi-threading for annotation scanning
Posted by GitBox <gi...@apache.org>.
martin-g commented on a change in pull request #354:
URL: https://github.com/apache/tomcat/pull/354#discussion_r486870063
##########
File path: java/org/apache/catalina/startup/LocalStrings_fr.properties
##########
@@ -69,6 +69,7 @@ contextConfig.missingRealm=Aucun royaume (realm) n'a été configuré pour réal
contextConfig.processAnnotationsDir.debug=Balayage du répertoire pour trouver des fichiers de classe avec annotations [{0}]
contextConfig.processAnnotationsJar.debug=Analyse du fichier jars pour des classes annotées avec [{0}]
contextConfig.processAnnotationsWebDir.debug=Balayage du répertoire d''applications web, pour fichiers de classe avec annotations [{0}]
+contextConfig.processAnnotationsInParallelFailure=exécution parallèle a échoué
Review comment:
Should `exécution` be capital cased like the other messages ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] kamnani commented on a change in pull request #354: Optimize Server startup time using multi-threading for annotation scanning
Posted by GitBox <gi...@apache.org>.
kamnani commented on a change in pull request #354:
URL: https://github.com/apache/tomcat/pull/354#discussion_r485129011
##########
File path: java/org/apache/catalina/mbeans/MBeanFactory.java
##########
@@ -490,6 +491,7 @@ public String createStandardContext(String parent,
public String createStandardHost(String parent, String name,
String appBase,
boolean autoDeploy,
+ boolean parallelAnnotationScanning,
Review comment:
Alright. I have removed these changes.
Resolved with the latest commit.
Thanks 👍
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] rmaucher closed pull request #354: Optimize Server startup time using multi-threading for annotation scanning
Posted by GitBox <gi...@apache.org>.
rmaucher closed pull request #354:
URL: https://github.com/apache/tomcat/pull/354
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] kamnani commented on a change in pull request #354: Optimize Server startup time using multi-threading for annotation scanning
Posted by GitBox <gi...@apache.org>.
kamnani commented on a change in pull request #354:
URL: https://github.com/apache/tomcat/pull/354#discussion_r489070396
##########
File path: java/org/apache/catalina/startup/LocalStrings_fr.properties
##########
@@ -69,6 +69,7 @@ contextConfig.missingRealm=Aucun royaume (realm) n'a été configuré pour réal
contextConfig.processAnnotationsDir.debug=Balayage du répertoire pour trouver des fichiers de classe avec annotations [{0}]
contextConfig.processAnnotationsJar.debug=Analyse du fichier jars pour des classes annotées avec [{0}]
contextConfig.processAnnotationsWebDir.debug=Balayage du répertoire d''applications web, pour fichiers de classe avec annotations [{0}]
+contextConfig.processAnnotationsInParallelFailure=exécution parallèle a échoué
Review comment:
Thanks for the suggestion.
Resolved with the latest commit.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] kamnani edited a comment on pull request #354: Optimize Server startup time using multi-threading for annotation scanning
Posted by GitBox <gi...@apache.org>.
kamnani edited a comment on pull request #354:
URL: https://github.com/apache/tomcat/pull/354#issuecomment-689023592
@rmaucher I actually tested the optimization with the flag in a large application having 1000's of Jar in the classpath and the server startup gains are significant and the stats are as follows :
2816 jars - 33% improvement
1800 jars - 30% improvement
1000 jars - 30% improvement
The default tomcat app shows an improvement of 6% with the flag.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] kamnani commented on pull request #354: Optimize Server startup time using multi-threading for annotation scanning
Posted by GitBox <gi...@apache.org>.
kamnani commented on pull request #354:
URL: https://github.com/apache/tomcat/pull/354#issuecomment-688967898
> I still don't really like some items, like the explicit flag (but I don't see yet how to do it) or some of the "optimizations".
> By default the utility thread count is 2, so not too much parallelism.
Yes, You're right with the default Value, but that can be changed through the Host Configuration.. right?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] rmaucher commented on a change in pull request #354: Optimize Server startup time using multi-threading for annotation scanning
Posted by GitBox <gi...@apache.org>.
rmaucher commented on a change in pull request #354:
URL: https://github.com/apache/tomcat/pull/354#discussion_r486924037
##########
File path: java/org/apache/catalina/startup/LocalStrings_fr.properties
##########
@@ -69,6 +69,7 @@ contextConfig.missingRealm=Aucun royaume (realm) n'a été configuré pour réal
contextConfig.processAnnotationsDir.debug=Balayage du répertoire pour trouver des fichiers de classe avec annotations [{0}]
contextConfig.processAnnotationsJar.debug=Analyse du fichier jars pour des classes annotées avec [{0}]
contextConfig.processAnnotationsWebDir.debug=Balayage du répertoire d''applications web, pour fichiers de classe avec annotations [{0}]
+contextConfig.processAnnotationsInParallelFailure=exécution parallèle a échoué
Review comment:
Yes, the translation is meh. As a rule, I'd say never commit any localization IMO, just let others handle in with poeditor later.
##########
File path: java/org/apache/catalina/startup/LocalStrings_fr.properties
##########
@@ -69,6 +69,7 @@ contextConfig.missingRealm=Aucun royaume (realm) n'a été configuré pour réal
contextConfig.processAnnotationsDir.debug=Balayage du répertoire pour trouver des fichiers de classe avec annotations [{0}]
contextConfig.processAnnotationsJar.debug=Analyse du fichier jars pour des classes annotées avec [{0}]
contextConfig.processAnnotationsWebDir.debug=Balayage du répertoire d''applications web, pour fichiers de classe avec annotations [{0}]
+contextConfig.processAnnotationsInParallelFailure=exécution parallèle a échoué
Review comment:
Yes, the translation is meh. As a rule, I'd say never commit any localization IMO, just let others handle in with poeditor later.
##########
File path: java/org/apache/catalina/startup/LocalStrings_fr.properties
##########
@@ -69,6 +69,7 @@ contextConfig.missingRealm=Aucun royaume (realm) n'a été configuré pour réal
contextConfig.processAnnotationsDir.debug=Balayage du répertoire pour trouver des fichiers de classe avec annotations [{0}]
contextConfig.processAnnotationsJar.debug=Analyse du fichier jars pour des classes annotées avec [{0}]
contextConfig.processAnnotationsWebDir.debug=Balayage du répertoire d''applications web, pour fichiers de classe avec annotations [{0}]
+contextConfig.processAnnotationsInParallelFailure=exécution parallèle a échoué
Review comment:
Yes, the translation is meh. As a rule, I'd say never commit any localization IMO, just let others handle in with poeditor later.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] kamnani commented on a change in pull request #354: Optimize Server startup time using multi-threading for annotation scanning
Posted by GitBox <gi...@apache.org>.
kamnani commented on a change in pull request #354:
URL: https://github.com/apache/tomcat/pull/354#discussion_r485071970
##########
File path: java/org/apache/catalina/startup/ContextConfig.java
##########
@@ -2136,26 +2157,98 @@ protected InputSource getWebXmlSource(String filename, boolean global) {
}
protected void processAnnotations(Set<WebXml> fragments,
- boolean handlesTypesOnly, Map<String,JavaClassCacheEntry> javaClassCache) {
- for(WebXml fragment : fragments) {
- // Only need to scan for @HandlesTypes matches if any of the
- // following are true:
- // - it has already been determined only @HandlesTypes is required
- // (e.g. main web.xml has metadata-complete="true"
- // - this fragment is for a container JAR (Servlet 3.1 section 8.1)
- // - this fragment has metadata-complete="true"
- boolean htOnly = handlesTypesOnly || !fragment.getWebappJar() ||
- fragment.isMetadataComplete();
-
- WebXml annotations = new WebXml();
- // no impact on distributable
- annotations.setDistributable(true);
- URL url = fragment.getURL();
- processAnnotationsUrl(url, annotations, htOnly, javaClassCache);
- Set<WebXml> set = new HashSet<>();
- set.add(annotations);
- // Merge annotations into fragment - fragment takes priority
- fragment.merge(set);
+ boolean handlesTypesOnly, Map<String, JavaClassCacheEntry> javaClassCache) {
+
+ if (context.getParent() instanceof Host && ((Host) context.getParent()).isParallelAnnotationScanning()) {
+ processAnnotationsInParallel(fragments, handlesTypesOnly, javaClassCache);
+ return;
+ }
+
+ for (WebXml fragment : fragments) {
+ scanWebXmlFragment(handlesTypesOnly, fragment, javaClassCache);
+ }
+ }
+
+ private void scanWebXmlFragment(boolean handlesTypesOnly, WebXml fragment, Map<String, JavaClassCacheEntry> javaClassCache) {
+
+ // Only need to scan for @HandlesTypes matches if any of the
+ // following are true:
+ // - it has already been determined only @HandlesTypes is required
+ // (e.g. main web.xml has metadata-complete="true"
+ // - this fragment is for a container JAR (Servlet 3.1 section 8.1)
+ // - this fragment has metadata-complete="true"
+ boolean htOnly = handlesTypesOnly || !fragment.getWebappJar() ||
+ fragment.isMetadataComplete();
+
+ WebXml annotations = new WebXml();
+ // no impact on distributable
+ annotations.setDistributable(true);
+ URL url = fragment.getURL();
+ processAnnotationsUrl(url, annotations, htOnly, javaClassCache);
+ Set<WebXml> set = new HashSet<>();
+ set.add(annotations);
+ // Merge annotations into fragment - fragment takes priority
+ fragment.merge(set);
+ }
+
+ /**
+ * Executable task to scan a segment for annotations. Each task does the
+ * same work as the for loop inside processAnnotations();
+ *
+ * @author Engebretson, John
+ * @author Kamnani, Jatin
+ */
+ private class AnnotationScanTask implements Callable<Void> {
+ private final WebXml fragment;
+ private final boolean handlesTypesOnly;
+ private Map<String, JavaClassCacheEntry> javaClassCache;
+
+ private AnnotationScanTask(WebXml fragment, boolean handlesTypesOnly, Map<String, JavaClassCacheEntry> javaClassCache) {
+ this.fragment = fragment;
+ this.handlesTypesOnly = handlesTypesOnly;
+ this.javaClassCache = javaClassCache;
+ }
+
+ @Override
+ public Void call() {
+ scanWebXmlFragment(handlesTypesOnly, fragment, javaClassCache);
+
+ return null;
+ }
+
+ }
+
+ /**
+ * Parallelized version of processAnnotationsInParallel(). Constructs tasks,
+ * submits them as they're created, then waits for completion.
+ *
+ * @param fragments Set of parallelizable scans
+ * @param handlesTypesOnly Important parameter for the underlying scan
+ */
+ protected void processAnnotationsInParallel(Set<WebXml> fragments, boolean handlesTypesOnly,
+ Map<String, JavaClassCacheEntry> javaClassCache) {
+
+
+ Server s = getServer();
+ ExecutorService pool = null;
+ try {
+ pool = s.getUtilityExecutor();
+ List<Future<Void>> futures = new ArrayList<>(fragments.size());
+ for (WebXml fragment : fragments) {
+ Callable<Void> task = new AnnotationScanTask(fragment, handlesTypesOnly, javaClassCache);
+ futures.add(pool.submit(task));
+ }
+ try {
+ for (Future<?> future : futures) {
+ future.get();
+ }
+ } catch (Exception e) {
+ throw new RuntimeException("Parallel execution failed", e);
Review comment:
Resolved with the latest changes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] rmaucher commented on pull request #354: Optimize Server startup time using multi-threading for annotation scanning
Posted by GitBox <gi...@apache.org>.
rmaucher commented on pull request #354:
URL: https://github.com/apache/tomcat/pull/354#issuecomment-687293425
I don't agree with this. There is a "utility" executor (Server.getUtilityExecutor()), it could be used for that instead of adding special purpose threads. Then you can add a flag to do the scanning as executor tasks or the usual way.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] martin-g commented on a change in pull request #354: Optimize Server startup time using multi-threading for annotation scanning
Posted by GitBox <gi...@apache.org>.
martin-g commented on a change in pull request #354:
URL: https://github.com/apache/tomcat/pull/354#discussion_r486870063
##########
File path: java/org/apache/catalina/startup/LocalStrings_fr.properties
##########
@@ -69,6 +69,7 @@ contextConfig.missingRealm=Aucun royaume (realm) n'a été configuré pour réal
contextConfig.processAnnotationsDir.debug=Balayage du répertoire pour trouver des fichiers de classe avec annotations [{0}]
contextConfig.processAnnotationsJar.debug=Analyse du fichier jars pour des classes annotées avec [{0}]
contextConfig.processAnnotationsWebDir.debug=Balayage du répertoire d''applications web, pour fichiers de classe avec annotations [{0}]
+contextConfig.processAnnotationsInParallelFailure=exécution parallèle a échoué
Review comment:
Should `exécution` be capital cased like the other messages ?
##########
File path: java/org/apache/catalina/startup/ContextConfig.java
##########
@@ -2136,26 +2146,98 @@ protected InputSource getWebXmlSource(String filename, boolean global) {
}
protected void processAnnotations(Set<WebXml> fragments,
- boolean handlesTypesOnly, Map<String,JavaClassCacheEntry> javaClassCache) {
- for(WebXml fragment : fragments) {
- // Only need to scan for @HandlesTypes matches if any of the
- // following are true:
- // - it has already been determined only @HandlesTypes is required
- // (e.g. main web.xml has metadata-complete="true"
- // - this fragment is for a container JAR (Servlet 3.1 section 8.1)
- // - this fragment has metadata-complete="true"
- boolean htOnly = handlesTypesOnly || !fragment.getWebappJar() ||
- fragment.isMetadataComplete();
-
- WebXml annotations = new WebXml();
- // no impact on distributable
- annotations.setDistributable(true);
- URL url = fragment.getURL();
- processAnnotationsUrl(url, annotations, htOnly, javaClassCache);
- Set<WebXml> set = new HashSet<>();
- set.add(annotations);
- // Merge annotations into fragment - fragment takes priority
- fragment.merge(set);
+ boolean handlesTypesOnly, Map<String, JavaClassCacheEntry> javaClassCache) {
+
+ if (context.getParent() instanceof Host && ((Host) context.getParent()).isParallelAnnotationScanning()) {
+ processAnnotationsInParallel(fragments, handlesTypesOnly, javaClassCache);
+ return;
+ }
+
+ for (WebXml fragment : fragments) {
+ scanWebXmlFragment(handlesTypesOnly, fragment, javaClassCache);
+ }
+ }
+
+ private void scanWebXmlFragment(boolean handlesTypesOnly, WebXml fragment, Map<String, JavaClassCacheEntry> javaClassCache) {
+
+ // Only need to scan for @HandlesTypes matches if any of the
+ // following are true:
+ // - it has already been determined only @HandlesTypes is required
+ // (e.g. main web.xml has metadata-complete="true"
+ // - this fragment is for a container JAR (Servlet 3.1 section 8.1)
+ // - this fragment has metadata-complete="true"
+ boolean htOnly = handlesTypesOnly || !fragment.getWebappJar() ||
+ fragment.isMetadataComplete();
+
+ WebXml annotations = new WebXml();
+ // no impact on distributable
+ annotations.setDistributable(true);
+ URL url = fragment.getURL();
+ processAnnotationsUrl(url, annotations, htOnly, javaClassCache);
+ Set<WebXml> set = new HashSet<>();
+ set.add(annotations);
+ // Merge annotations into fragment - fragment takes priority
+ fragment.merge(set);
+ }
+
+ /**
+ * Executable task to scan a segment for annotations. Each task does the
+ * same work as the for loop inside processAnnotations();
+ *
+ * @author Engebretson, John
+ * @author Kamnani, Jatin
+ */
+ private class AnnotationScanTask implements Callable<Void> {
+ private final WebXml fragment;
+ private final boolean handlesTypesOnly;
+ private Map<String, JavaClassCacheEntry> javaClassCache;
+
+ private AnnotationScanTask(WebXml fragment, boolean handlesTypesOnly, Map<String, JavaClassCacheEntry> javaClassCache) {
+ this.fragment = fragment;
+ this.handlesTypesOnly = handlesTypesOnly;
+ this.javaClassCache = javaClassCache;
+ }
+
+ @Override
+ public Void call() {
+ scanWebXmlFragment(handlesTypesOnly, fragment, javaClassCache);
+
+ return null;
+ }
+
+ }
+
+ /**
+ * Parallelized version of processAnnotationsInParallel(). Constructs tasks,
+ * submits them as they're created, then waits for completion.
+ *
+ * @param fragments Set of parallelizable scans
+ * @param handlesTypesOnly Important parameter for the underlying scan
+ */
+ protected void processAnnotationsInParallel(Set<WebXml> fragments, boolean handlesTypesOnly,
+ Map<String, JavaClassCacheEntry> javaClassCache) {
+
+
+ Server s = getServer();
+ ExecutorService pool = null;
+ try {
+ pool = s.getUtilityExecutor();
+ List<Future<Void>> futures = new ArrayList<>(fragments.size());
+ for (WebXml fragment : fragments) {
+ Callable<Void> task = new AnnotationScanTask(fragment, handlesTypesOnly, javaClassCache);
+ futures.add(pool.submit(task));
+ }
+ try {
+ for (Future<?> future : futures) {
+ future.get();
+ }
+ } catch (Exception e) {
+ throw new RuntimeException(sm.getString("contextConfig.processAnnotationsInParallelFailure"), e);
+ }
+ } finally {
+ if (pool != null) {
+ pool.shutdownNow();
Review comment:
This is not really needed because org.apache.tomcat.util.threads.ScheduledThreadPoolExecutor#shutdown methods do nothing, but anyway it is a good practice to do it.
##########
File path: java/org/apache/catalina/startup/ContextConfig.java
##########
@@ -2136,26 +2146,98 @@ protected InputSource getWebXmlSource(String filename, boolean global) {
}
protected void processAnnotations(Set<WebXml> fragments,
- boolean handlesTypesOnly, Map<String,JavaClassCacheEntry> javaClassCache) {
- for(WebXml fragment : fragments) {
- // Only need to scan for @HandlesTypes matches if any of the
- // following are true:
- // - it has already been determined only @HandlesTypes is required
- // (e.g. main web.xml has metadata-complete="true"
- // - this fragment is for a container JAR (Servlet 3.1 section 8.1)
- // - this fragment has metadata-complete="true"
- boolean htOnly = handlesTypesOnly || !fragment.getWebappJar() ||
- fragment.isMetadataComplete();
-
- WebXml annotations = new WebXml();
- // no impact on distributable
- annotations.setDistributable(true);
- URL url = fragment.getURL();
- processAnnotationsUrl(url, annotations, htOnly, javaClassCache);
- Set<WebXml> set = new HashSet<>();
- set.add(annotations);
- // Merge annotations into fragment - fragment takes priority
- fragment.merge(set);
+ boolean handlesTypesOnly, Map<String, JavaClassCacheEntry> javaClassCache) {
+
+ if (context.getParent() instanceof Host && ((Host) context.getParent()).isParallelAnnotationScanning()) {
+ processAnnotationsInParallel(fragments, handlesTypesOnly, javaClassCache);
+ return;
+ }
+
+ for (WebXml fragment : fragments) {
+ scanWebXmlFragment(handlesTypesOnly, fragment, javaClassCache);
+ }
+ }
+
+ private void scanWebXmlFragment(boolean handlesTypesOnly, WebXml fragment, Map<String, JavaClassCacheEntry> javaClassCache) {
+
+ // Only need to scan for @HandlesTypes matches if any of the
+ // following are true:
+ // - it has already been determined only @HandlesTypes is required
+ // (e.g. main web.xml has metadata-complete="true"
+ // - this fragment is for a container JAR (Servlet 3.1 section 8.1)
+ // - this fragment has metadata-complete="true"
+ boolean htOnly = handlesTypesOnly || !fragment.getWebappJar() ||
+ fragment.isMetadataComplete();
+
+ WebXml annotations = new WebXml();
+ // no impact on distributable
+ annotations.setDistributable(true);
+ URL url = fragment.getURL();
+ processAnnotationsUrl(url, annotations, htOnly, javaClassCache);
+ Set<WebXml> set = new HashSet<>();
+ set.add(annotations);
+ // Merge annotations into fragment - fragment takes priority
+ fragment.merge(set);
+ }
+
+ /**
+ * Executable task to scan a segment for annotations. Each task does the
+ * same work as the for loop inside processAnnotations();
+ *
+ * @author Engebretson, John
+ * @author Kamnani, Jatin
+ */
+ private class AnnotationScanTask implements Callable<Void> {
Review comment:
Why `Callable<Void>` ?
You can use `Runnable`
##########
File path: java/org/apache/catalina/startup/LocalStrings_fr.properties
##########
@@ -69,6 +69,7 @@ contextConfig.missingRealm=Aucun royaume (realm) n'a été configuré pour réal
contextConfig.processAnnotationsDir.debug=Balayage du répertoire pour trouver des fichiers de classe avec annotations [{0}]
contextConfig.processAnnotationsJar.debug=Analyse du fichier jars pour des classes annotées avec [{0}]
contextConfig.processAnnotationsWebDir.debug=Balayage du répertoire d''applications web, pour fichiers de classe avec annotations [{0}]
+contextConfig.processAnnotationsInParallelFailure=exécution parallèle a échoué
Review comment:
Should `exécution` be capital cased like the other messages ?
##########
File path: java/org/apache/catalina/startup/ContextConfig.java
##########
@@ -2136,26 +2146,98 @@ protected InputSource getWebXmlSource(String filename, boolean global) {
}
protected void processAnnotations(Set<WebXml> fragments,
- boolean handlesTypesOnly, Map<String,JavaClassCacheEntry> javaClassCache) {
- for(WebXml fragment : fragments) {
- // Only need to scan for @HandlesTypes matches if any of the
- // following are true:
- // - it has already been determined only @HandlesTypes is required
- // (e.g. main web.xml has metadata-complete="true"
- // - this fragment is for a container JAR (Servlet 3.1 section 8.1)
- // - this fragment has metadata-complete="true"
- boolean htOnly = handlesTypesOnly || !fragment.getWebappJar() ||
- fragment.isMetadataComplete();
-
- WebXml annotations = new WebXml();
- // no impact on distributable
- annotations.setDistributable(true);
- URL url = fragment.getURL();
- processAnnotationsUrl(url, annotations, htOnly, javaClassCache);
- Set<WebXml> set = new HashSet<>();
- set.add(annotations);
- // Merge annotations into fragment - fragment takes priority
- fragment.merge(set);
+ boolean handlesTypesOnly, Map<String, JavaClassCacheEntry> javaClassCache) {
+
+ if (context.getParent() instanceof Host && ((Host) context.getParent()).isParallelAnnotationScanning()) {
+ processAnnotationsInParallel(fragments, handlesTypesOnly, javaClassCache);
+ return;
+ }
+
+ for (WebXml fragment : fragments) {
+ scanWebXmlFragment(handlesTypesOnly, fragment, javaClassCache);
+ }
+ }
+
+ private void scanWebXmlFragment(boolean handlesTypesOnly, WebXml fragment, Map<String, JavaClassCacheEntry> javaClassCache) {
+
+ // Only need to scan for @HandlesTypes matches if any of the
+ // following are true:
+ // - it has already been determined only @HandlesTypes is required
+ // (e.g. main web.xml has metadata-complete="true"
+ // - this fragment is for a container JAR (Servlet 3.1 section 8.1)
+ // - this fragment has metadata-complete="true"
+ boolean htOnly = handlesTypesOnly || !fragment.getWebappJar() ||
+ fragment.isMetadataComplete();
+
+ WebXml annotations = new WebXml();
+ // no impact on distributable
+ annotations.setDistributable(true);
+ URL url = fragment.getURL();
+ processAnnotationsUrl(url, annotations, htOnly, javaClassCache);
+ Set<WebXml> set = new HashSet<>();
+ set.add(annotations);
+ // Merge annotations into fragment - fragment takes priority
+ fragment.merge(set);
+ }
+
+ /**
+ * Executable task to scan a segment for annotations. Each task does the
+ * same work as the for loop inside processAnnotations();
+ *
+ * @author Engebretson, John
+ * @author Kamnani, Jatin
+ */
+ private class AnnotationScanTask implements Callable<Void> {
+ private final WebXml fragment;
+ private final boolean handlesTypesOnly;
+ private Map<String, JavaClassCacheEntry> javaClassCache;
+
+ private AnnotationScanTask(WebXml fragment, boolean handlesTypesOnly, Map<String, JavaClassCacheEntry> javaClassCache) {
+ this.fragment = fragment;
+ this.handlesTypesOnly = handlesTypesOnly;
+ this.javaClassCache = javaClassCache;
+ }
+
+ @Override
+ public Void call() {
+ scanWebXmlFragment(handlesTypesOnly, fragment, javaClassCache);
+
+ return null;
+ }
+
+ }
+
+ /**
+ * Parallelized version of processAnnotationsInParallel(). Constructs tasks,
+ * submits them as they're created, then waits for completion.
+ *
+ * @param fragments Set of parallelizable scans
+ * @param handlesTypesOnly Important parameter for the underlying scan
+ */
+ protected void processAnnotationsInParallel(Set<WebXml> fragments, boolean handlesTypesOnly,
+ Map<String, JavaClassCacheEntry> javaClassCache) {
+
+
+ Server s = getServer();
+ ExecutorService pool = null;
+ try {
+ pool = s.getUtilityExecutor();
+ List<Future<Void>> futures = new ArrayList<>(fragments.size());
+ for (WebXml fragment : fragments) {
+ Callable<Void> task = new AnnotationScanTask(fragment, handlesTypesOnly, javaClassCache);
+ futures.add(pool.submit(task));
+ }
+ try {
+ for (Future<?> future : futures) {
+ future.get();
+ }
+ } catch (Exception e) {
+ throw new RuntimeException(sm.getString("contextConfig.processAnnotationsInParallelFailure"), e);
+ }
+ } finally {
+ if (pool != null) {
+ pool.shutdownNow();
Review comment:
This is not really needed because org.apache.tomcat.util.threads.ScheduledThreadPoolExecutor#shutdown methods do nothing, but anyway it is a good practice to do it.
##########
File path: java/org/apache/catalina/startup/ContextConfig.java
##########
@@ -2136,26 +2146,98 @@ protected InputSource getWebXmlSource(String filename, boolean global) {
}
protected void processAnnotations(Set<WebXml> fragments,
- boolean handlesTypesOnly, Map<String,JavaClassCacheEntry> javaClassCache) {
- for(WebXml fragment : fragments) {
- // Only need to scan for @HandlesTypes matches if any of the
- // following are true:
- // - it has already been determined only @HandlesTypes is required
- // (e.g. main web.xml has metadata-complete="true"
- // - this fragment is for a container JAR (Servlet 3.1 section 8.1)
- // - this fragment has metadata-complete="true"
- boolean htOnly = handlesTypesOnly || !fragment.getWebappJar() ||
- fragment.isMetadataComplete();
-
- WebXml annotations = new WebXml();
- // no impact on distributable
- annotations.setDistributable(true);
- URL url = fragment.getURL();
- processAnnotationsUrl(url, annotations, htOnly, javaClassCache);
- Set<WebXml> set = new HashSet<>();
- set.add(annotations);
- // Merge annotations into fragment - fragment takes priority
- fragment.merge(set);
+ boolean handlesTypesOnly, Map<String, JavaClassCacheEntry> javaClassCache) {
+
+ if (context.getParent() instanceof Host && ((Host) context.getParent()).isParallelAnnotationScanning()) {
+ processAnnotationsInParallel(fragments, handlesTypesOnly, javaClassCache);
+ return;
+ }
+
+ for (WebXml fragment : fragments) {
+ scanWebXmlFragment(handlesTypesOnly, fragment, javaClassCache);
+ }
+ }
+
+ private void scanWebXmlFragment(boolean handlesTypesOnly, WebXml fragment, Map<String, JavaClassCacheEntry> javaClassCache) {
+
+ // Only need to scan for @HandlesTypes matches if any of the
+ // following are true:
+ // - it has already been determined only @HandlesTypes is required
+ // (e.g. main web.xml has metadata-complete="true"
+ // - this fragment is for a container JAR (Servlet 3.1 section 8.1)
+ // - this fragment has metadata-complete="true"
+ boolean htOnly = handlesTypesOnly || !fragment.getWebappJar() ||
+ fragment.isMetadataComplete();
+
+ WebXml annotations = new WebXml();
+ // no impact on distributable
+ annotations.setDistributable(true);
+ URL url = fragment.getURL();
+ processAnnotationsUrl(url, annotations, htOnly, javaClassCache);
+ Set<WebXml> set = new HashSet<>();
+ set.add(annotations);
+ // Merge annotations into fragment - fragment takes priority
+ fragment.merge(set);
+ }
+
+ /**
+ * Executable task to scan a segment for annotations. Each task does the
+ * same work as the for loop inside processAnnotations();
+ *
+ * @author Engebretson, John
+ * @author Kamnani, Jatin
+ */
+ private class AnnotationScanTask implements Callable<Void> {
Review comment:
Why `Callable<Void>` ?
You can use `Runnable`
##########
File path: java/org/apache/catalina/startup/LocalStrings_fr.properties
##########
@@ -69,6 +69,7 @@ contextConfig.missingRealm=Aucun royaume (realm) n'a été configuré pour réal
contextConfig.processAnnotationsDir.debug=Balayage du répertoire pour trouver des fichiers de classe avec annotations [{0}]
contextConfig.processAnnotationsJar.debug=Analyse du fichier jars pour des classes annotées avec [{0}]
contextConfig.processAnnotationsWebDir.debug=Balayage du répertoire d''applications web, pour fichiers de classe avec annotations [{0}]
+contextConfig.processAnnotationsInParallelFailure=exécution parallèle a échoué
Review comment:
Should `exécution` be capital cased like the other messages ?
##########
File path: java/org/apache/catalina/startup/ContextConfig.java
##########
@@ -2136,26 +2146,98 @@ protected InputSource getWebXmlSource(String filename, boolean global) {
}
protected void processAnnotations(Set<WebXml> fragments,
- boolean handlesTypesOnly, Map<String,JavaClassCacheEntry> javaClassCache) {
- for(WebXml fragment : fragments) {
- // Only need to scan for @HandlesTypes matches if any of the
- // following are true:
- // - it has already been determined only @HandlesTypes is required
- // (e.g. main web.xml has metadata-complete="true"
- // - this fragment is for a container JAR (Servlet 3.1 section 8.1)
- // - this fragment has metadata-complete="true"
- boolean htOnly = handlesTypesOnly || !fragment.getWebappJar() ||
- fragment.isMetadataComplete();
-
- WebXml annotations = new WebXml();
- // no impact on distributable
- annotations.setDistributable(true);
- URL url = fragment.getURL();
- processAnnotationsUrl(url, annotations, htOnly, javaClassCache);
- Set<WebXml> set = new HashSet<>();
- set.add(annotations);
- // Merge annotations into fragment - fragment takes priority
- fragment.merge(set);
+ boolean handlesTypesOnly, Map<String, JavaClassCacheEntry> javaClassCache) {
+
+ if (context.getParent() instanceof Host && ((Host) context.getParent()).isParallelAnnotationScanning()) {
+ processAnnotationsInParallel(fragments, handlesTypesOnly, javaClassCache);
+ return;
+ }
+
+ for (WebXml fragment : fragments) {
+ scanWebXmlFragment(handlesTypesOnly, fragment, javaClassCache);
+ }
+ }
+
+ private void scanWebXmlFragment(boolean handlesTypesOnly, WebXml fragment, Map<String, JavaClassCacheEntry> javaClassCache) {
+
+ // Only need to scan for @HandlesTypes matches if any of the
+ // following are true:
+ // - it has already been determined only @HandlesTypes is required
+ // (e.g. main web.xml has metadata-complete="true"
+ // - this fragment is for a container JAR (Servlet 3.1 section 8.1)
+ // - this fragment has metadata-complete="true"
+ boolean htOnly = handlesTypesOnly || !fragment.getWebappJar() ||
+ fragment.isMetadataComplete();
+
+ WebXml annotations = new WebXml();
+ // no impact on distributable
+ annotations.setDistributable(true);
+ URL url = fragment.getURL();
+ processAnnotationsUrl(url, annotations, htOnly, javaClassCache);
+ Set<WebXml> set = new HashSet<>();
+ set.add(annotations);
+ // Merge annotations into fragment - fragment takes priority
+ fragment.merge(set);
+ }
+
+ /**
+ * Executable task to scan a segment for annotations. Each task does the
+ * same work as the for loop inside processAnnotations();
+ *
+ * @author Engebretson, John
+ * @author Kamnani, Jatin
+ */
+ private class AnnotationScanTask implements Callable<Void> {
+ private final WebXml fragment;
+ private final boolean handlesTypesOnly;
+ private Map<String, JavaClassCacheEntry> javaClassCache;
+
+ private AnnotationScanTask(WebXml fragment, boolean handlesTypesOnly, Map<String, JavaClassCacheEntry> javaClassCache) {
+ this.fragment = fragment;
+ this.handlesTypesOnly = handlesTypesOnly;
+ this.javaClassCache = javaClassCache;
+ }
+
+ @Override
+ public Void call() {
+ scanWebXmlFragment(handlesTypesOnly, fragment, javaClassCache);
+
+ return null;
+ }
+
+ }
+
+ /**
+ * Parallelized version of processAnnotationsInParallel(). Constructs tasks,
+ * submits them as they're created, then waits for completion.
+ *
+ * @param fragments Set of parallelizable scans
+ * @param handlesTypesOnly Important parameter for the underlying scan
+ */
+ protected void processAnnotationsInParallel(Set<WebXml> fragments, boolean handlesTypesOnly,
+ Map<String, JavaClassCacheEntry> javaClassCache) {
+
+
+ Server s = getServer();
+ ExecutorService pool = null;
+ try {
+ pool = s.getUtilityExecutor();
+ List<Future<Void>> futures = new ArrayList<>(fragments.size());
+ for (WebXml fragment : fragments) {
+ Callable<Void> task = new AnnotationScanTask(fragment, handlesTypesOnly, javaClassCache);
+ futures.add(pool.submit(task));
+ }
+ try {
+ for (Future<?> future : futures) {
+ future.get();
+ }
+ } catch (Exception e) {
+ throw new RuntimeException(sm.getString("contextConfig.processAnnotationsInParallelFailure"), e);
+ }
+ } finally {
+ if (pool != null) {
+ pool.shutdownNow();
Review comment:
This is not really needed because org.apache.tomcat.util.threads.ScheduledThreadPoolExecutor#shutdown methods do nothing, but anyway it is a good practice to do it.
##########
File path: java/org/apache/catalina/startup/ContextConfig.java
##########
@@ -2136,26 +2146,98 @@ protected InputSource getWebXmlSource(String filename, boolean global) {
}
protected void processAnnotations(Set<WebXml> fragments,
- boolean handlesTypesOnly, Map<String,JavaClassCacheEntry> javaClassCache) {
- for(WebXml fragment : fragments) {
- // Only need to scan for @HandlesTypes matches if any of the
- // following are true:
- // - it has already been determined only @HandlesTypes is required
- // (e.g. main web.xml has metadata-complete="true"
- // - this fragment is for a container JAR (Servlet 3.1 section 8.1)
- // - this fragment has metadata-complete="true"
- boolean htOnly = handlesTypesOnly || !fragment.getWebappJar() ||
- fragment.isMetadataComplete();
-
- WebXml annotations = new WebXml();
- // no impact on distributable
- annotations.setDistributable(true);
- URL url = fragment.getURL();
- processAnnotationsUrl(url, annotations, htOnly, javaClassCache);
- Set<WebXml> set = new HashSet<>();
- set.add(annotations);
- // Merge annotations into fragment - fragment takes priority
- fragment.merge(set);
+ boolean handlesTypesOnly, Map<String, JavaClassCacheEntry> javaClassCache) {
+
+ if (context.getParent() instanceof Host && ((Host) context.getParent()).isParallelAnnotationScanning()) {
+ processAnnotationsInParallel(fragments, handlesTypesOnly, javaClassCache);
+ return;
+ }
+
+ for (WebXml fragment : fragments) {
+ scanWebXmlFragment(handlesTypesOnly, fragment, javaClassCache);
+ }
+ }
+
+ private void scanWebXmlFragment(boolean handlesTypesOnly, WebXml fragment, Map<String, JavaClassCacheEntry> javaClassCache) {
+
+ // Only need to scan for @HandlesTypes matches if any of the
+ // following are true:
+ // - it has already been determined only @HandlesTypes is required
+ // (e.g. main web.xml has metadata-complete="true"
+ // - this fragment is for a container JAR (Servlet 3.1 section 8.1)
+ // - this fragment has metadata-complete="true"
+ boolean htOnly = handlesTypesOnly || !fragment.getWebappJar() ||
+ fragment.isMetadataComplete();
+
+ WebXml annotations = new WebXml();
+ // no impact on distributable
+ annotations.setDistributable(true);
+ URL url = fragment.getURL();
+ processAnnotationsUrl(url, annotations, htOnly, javaClassCache);
+ Set<WebXml> set = new HashSet<>();
+ set.add(annotations);
+ // Merge annotations into fragment - fragment takes priority
+ fragment.merge(set);
+ }
+
+ /**
+ * Executable task to scan a segment for annotations. Each task does the
+ * same work as the for loop inside processAnnotations();
+ *
+ * @author Engebretson, John
+ * @author Kamnani, Jatin
+ */
+ private class AnnotationScanTask implements Callable<Void> {
Review comment:
Why `Callable<Void>` ?
You can use `Runnable`
##########
File path: java/org/apache/catalina/startup/LocalStrings_fr.properties
##########
@@ -69,6 +69,7 @@ contextConfig.missingRealm=Aucun royaume (realm) n'a été configuré pour réal
contextConfig.processAnnotationsDir.debug=Balayage du répertoire pour trouver des fichiers de classe avec annotations [{0}]
contextConfig.processAnnotationsJar.debug=Analyse du fichier jars pour des classes annotées avec [{0}]
contextConfig.processAnnotationsWebDir.debug=Balayage du répertoire d''applications web, pour fichiers de classe avec annotations [{0}]
+contextConfig.processAnnotationsInParallelFailure=exécution parallèle a échoué
Review comment:
Should `exécution` be capital cased like the other messages ?
##########
File path: java/org/apache/catalina/startup/ContextConfig.java
##########
@@ -2136,26 +2146,98 @@ protected InputSource getWebXmlSource(String filename, boolean global) {
}
protected void processAnnotations(Set<WebXml> fragments,
- boolean handlesTypesOnly, Map<String,JavaClassCacheEntry> javaClassCache) {
- for(WebXml fragment : fragments) {
- // Only need to scan for @HandlesTypes matches if any of the
- // following are true:
- // - it has already been determined only @HandlesTypes is required
- // (e.g. main web.xml has metadata-complete="true"
- // - this fragment is for a container JAR (Servlet 3.1 section 8.1)
- // - this fragment has metadata-complete="true"
- boolean htOnly = handlesTypesOnly || !fragment.getWebappJar() ||
- fragment.isMetadataComplete();
-
- WebXml annotations = new WebXml();
- // no impact on distributable
- annotations.setDistributable(true);
- URL url = fragment.getURL();
- processAnnotationsUrl(url, annotations, htOnly, javaClassCache);
- Set<WebXml> set = new HashSet<>();
- set.add(annotations);
- // Merge annotations into fragment - fragment takes priority
- fragment.merge(set);
+ boolean handlesTypesOnly, Map<String, JavaClassCacheEntry> javaClassCache) {
+
+ if (context.getParent() instanceof Host && ((Host) context.getParent()).isParallelAnnotationScanning()) {
+ processAnnotationsInParallel(fragments, handlesTypesOnly, javaClassCache);
+ return;
+ }
+
+ for (WebXml fragment : fragments) {
+ scanWebXmlFragment(handlesTypesOnly, fragment, javaClassCache);
+ }
+ }
+
+ private void scanWebXmlFragment(boolean handlesTypesOnly, WebXml fragment, Map<String, JavaClassCacheEntry> javaClassCache) {
+
+ // Only need to scan for @HandlesTypes matches if any of the
+ // following are true:
+ // - it has already been determined only @HandlesTypes is required
+ // (e.g. main web.xml has metadata-complete="true"
+ // - this fragment is for a container JAR (Servlet 3.1 section 8.1)
+ // - this fragment has metadata-complete="true"
+ boolean htOnly = handlesTypesOnly || !fragment.getWebappJar() ||
+ fragment.isMetadataComplete();
+
+ WebXml annotations = new WebXml();
+ // no impact on distributable
+ annotations.setDistributable(true);
+ URL url = fragment.getURL();
+ processAnnotationsUrl(url, annotations, htOnly, javaClassCache);
+ Set<WebXml> set = new HashSet<>();
+ set.add(annotations);
+ // Merge annotations into fragment - fragment takes priority
+ fragment.merge(set);
+ }
+
+ /**
+ * Executable task to scan a segment for annotations. Each task does the
+ * same work as the for loop inside processAnnotations();
+ *
+ * @author Engebretson, John
+ * @author Kamnani, Jatin
+ */
+ private class AnnotationScanTask implements Callable<Void> {
+ private final WebXml fragment;
+ private final boolean handlesTypesOnly;
+ private Map<String, JavaClassCacheEntry> javaClassCache;
+
+ private AnnotationScanTask(WebXml fragment, boolean handlesTypesOnly, Map<String, JavaClassCacheEntry> javaClassCache) {
+ this.fragment = fragment;
+ this.handlesTypesOnly = handlesTypesOnly;
+ this.javaClassCache = javaClassCache;
+ }
+
+ @Override
+ public Void call() {
+ scanWebXmlFragment(handlesTypesOnly, fragment, javaClassCache);
+
+ return null;
+ }
+
+ }
+
+ /**
+ * Parallelized version of processAnnotationsInParallel(). Constructs tasks,
+ * submits them as they're created, then waits for completion.
+ *
+ * @param fragments Set of parallelizable scans
+ * @param handlesTypesOnly Important parameter for the underlying scan
+ */
+ protected void processAnnotationsInParallel(Set<WebXml> fragments, boolean handlesTypesOnly,
+ Map<String, JavaClassCacheEntry> javaClassCache) {
+
+
+ Server s = getServer();
+ ExecutorService pool = null;
+ try {
+ pool = s.getUtilityExecutor();
+ List<Future<Void>> futures = new ArrayList<>(fragments.size());
+ for (WebXml fragment : fragments) {
+ Callable<Void> task = new AnnotationScanTask(fragment, handlesTypesOnly, javaClassCache);
+ futures.add(pool.submit(task));
+ }
+ try {
+ for (Future<?> future : futures) {
+ future.get();
+ }
+ } catch (Exception e) {
+ throw new RuntimeException(sm.getString("contextConfig.processAnnotationsInParallelFailure"), e);
+ }
+ } finally {
+ if (pool != null) {
+ pool.shutdownNow();
Review comment:
This is not really needed because org.apache.tomcat.util.threads.ScheduledThreadPoolExecutor#shutdown methods do nothing, but anyway it is a good practice to do it.
##########
File path: java/org/apache/catalina/startup/ContextConfig.java
##########
@@ -2136,26 +2146,98 @@ protected InputSource getWebXmlSource(String filename, boolean global) {
}
protected void processAnnotations(Set<WebXml> fragments,
- boolean handlesTypesOnly, Map<String,JavaClassCacheEntry> javaClassCache) {
- for(WebXml fragment : fragments) {
- // Only need to scan for @HandlesTypes matches if any of the
- // following are true:
- // - it has already been determined only @HandlesTypes is required
- // (e.g. main web.xml has metadata-complete="true"
- // - this fragment is for a container JAR (Servlet 3.1 section 8.1)
- // - this fragment has metadata-complete="true"
- boolean htOnly = handlesTypesOnly || !fragment.getWebappJar() ||
- fragment.isMetadataComplete();
-
- WebXml annotations = new WebXml();
- // no impact on distributable
- annotations.setDistributable(true);
- URL url = fragment.getURL();
- processAnnotationsUrl(url, annotations, htOnly, javaClassCache);
- Set<WebXml> set = new HashSet<>();
- set.add(annotations);
- // Merge annotations into fragment - fragment takes priority
- fragment.merge(set);
+ boolean handlesTypesOnly, Map<String, JavaClassCacheEntry> javaClassCache) {
+
+ if (context.getParent() instanceof Host && ((Host) context.getParent()).isParallelAnnotationScanning()) {
+ processAnnotationsInParallel(fragments, handlesTypesOnly, javaClassCache);
+ return;
+ }
+
+ for (WebXml fragment : fragments) {
+ scanWebXmlFragment(handlesTypesOnly, fragment, javaClassCache);
+ }
+ }
+
+ private void scanWebXmlFragment(boolean handlesTypesOnly, WebXml fragment, Map<String, JavaClassCacheEntry> javaClassCache) {
+
+ // Only need to scan for @HandlesTypes matches if any of the
+ // following are true:
+ // - it has already been determined only @HandlesTypes is required
+ // (e.g. main web.xml has metadata-complete="true"
+ // - this fragment is for a container JAR (Servlet 3.1 section 8.1)
+ // - this fragment has metadata-complete="true"
+ boolean htOnly = handlesTypesOnly || !fragment.getWebappJar() ||
+ fragment.isMetadataComplete();
+
+ WebXml annotations = new WebXml();
+ // no impact on distributable
+ annotations.setDistributable(true);
+ URL url = fragment.getURL();
+ processAnnotationsUrl(url, annotations, htOnly, javaClassCache);
+ Set<WebXml> set = new HashSet<>();
+ set.add(annotations);
+ // Merge annotations into fragment - fragment takes priority
+ fragment.merge(set);
+ }
+
+ /**
+ * Executable task to scan a segment for annotations. Each task does the
+ * same work as the for loop inside processAnnotations();
+ *
+ * @author Engebretson, John
+ * @author Kamnani, Jatin
+ */
+ private class AnnotationScanTask implements Callable<Void> {
Review comment:
Why `Callable<Void>` ?
You can use `Runnable`
##########
File path: java/org/apache/catalina/startup/LocalStrings_fr.properties
##########
@@ -69,6 +69,7 @@ contextConfig.missingRealm=Aucun royaume (realm) n'a été configuré pour réal
contextConfig.processAnnotationsDir.debug=Balayage du répertoire pour trouver des fichiers de classe avec annotations [{0}]
contextConfig.processAnnotationsJar.debug=Analyse du fichier jars pour des classes annotées avec [{0}]
contextConfig.processAnnotationsWebDir.debug=Balayage du répertoire d''applications web, pour fichiers de classe avec annotations [{0}]
+contextConfig.processAnnotationsInParallelFailure=exécution parallèle a échoué
Review comment:
Should `exécution` be capital cased like the other messages ?
##########
File path: java/org/apache/catalina/startup/ContextConfig.java
##########
@@ -2136,26 +2146,98 @@ protected InputSource getWebXmlSource(String filename, boolean global) {
}
protected void processAnnotations(Set<WebXml> fragments,
- boolean handlesTypesOnly, Map<String,JavaClassCacheEntry> javaClassCache) {
- for(WebXml fragment : fragments) {
- // Only need to scan for @HandlesTypes matches if any of the
- // following are true:
- // - it has already been determined only @HandlesTypes is required
- // (e.g. main web.xml has metadata-complete="true"
- // - this fragment is for a container JAR (Servlet 3.1 section 8.1)
- // - this fragment has metadata-complete="true"
- boolean htOnly = handlesTypesOnly || !fragment.getWebappJar() ||
- fragment.isMetadataComplete();
-
- WebXml annotations = new WebXml();
- // no impact on distributable
- annotations.setDistributable(true);
- URL url = fragment.getURL();
- processAnnotationsUrl(url, annotations, htOnly, javaClassCache);
- Set<WebXml> set = new HashSet<>();
- set.add(annotations);
- // Merge annotations into fragment - fragment takes priority
- fragment.merge(set);
+ boolean handlesTypesOnly, Map<String, JavaClassCacheEntry> javaClassCache) {
+
+ if (context.getParent() instanceof Host && ((Host) context.getParent()).isParallelAnnotationScanning()) {
+ processAnnotationsInParallel(fragments, handlesTypesOnly, javaClassCache);
+ return;
+ }
+
+ for (WebXml fragment : fragments) {
+ scanWebXmlFragment(handlesTypesOnly, fragment, javaClassCache);
+ }
+ }
+
+ private void scanWebXmlFragment(boolean handlesTypesOnly, WebXml fragment, Map<String, JavaClassCacheEntry> javaClassCache) {
+
+ // Only need to scan for @HandlesTypes matches if any of the
+ // following are true:
+ // - it has already been determined only @HandlesTypes is required
+ // (e.g. main web.xml has metadata-complete="true"
+ // - this fragment is for a container JAR (Servlet 3.1 section 8.1)
+ // - this fragment has metadata-complete="true"
+ boolean htOnly = handlesTypesOnly || !fragment.getWebappJar() ||
+ fragment.isMetadataComplete();
+
+ WebXml annotations = new WebXml();
+ // no impact on distributable
+ annotations.setDistributable(true);
+ URL url = fragment.getURL();
+ processAnnotationsUrl(url, annotations, htOnly, javaClassCache);
+ Set<WebXml> set = new HashSet<>();
+ set.add(annotations);
+ // Merge annotations into fragment - fragment takes priority
+ fragment.merge(set);
+ }
+
+ /**
+ * Executable task to scan a segment for annotations. Each task does the
+ * same work as the for loop inside processAnnotations();
+ *
+ * @author Engebretson, John
+ * @author Kamnani, Jatin
+ */
+ private class AnnotationScanTask implements Callable<Void> {
+ private final WebXml fragment;
+ private final boolean handlesTypesOnly;
+ private Map<String, JavaClassCacheEntry> javaClassCache;
+
+ private AnnotationScanTask(WebXml fragment, boolean handlesTypesOnly, Map<String, JavaClassCacheEntry> javaClassCache) {
+ this.fragment = fragment;
+ this.handlesTypesOnly = handlesTypesOnly;
+ this.javaClassCache = javaClassCache;
+ }
+
+ @Override
+ public Void call() {
+ scanWebXmlFragment(handlesTypesOnly, fragment, javaClassCache);
+
+ return null;
+ }
+
+ }
+
+ /**
+ * Parallelized version of processAnnotationsInParallel(). Constructs tasks,
+ * submits them as they're created, then waits for completion.
+ *
+ * @param fragments Set of parallelizable scans
+ * @param handlesTypesOnly Important parameter for the underlying scan
+ */
+ protected void processAnnotationsInParallel(Set<WebXml> fragments, boolean handlesTypesOnly,
+ Map<String, JavaClassCacheEntry> javaClassCache) {
+
+
+ Server s = getServer();
+ ExecutorService pool = null;
+ try {
+ pool = s.getUtilityExecutor();
+ List<Future<Void>> futures = new ArrayList<>(fragments.size());
+ for (WebXml fragment : fragments) {
+ Callable<Void> task = new AnnotationScanTask(fragment, handlesTypesOnly, javaClassCache);
+ futures.add(pool.submit(task));
+ }
+ try {
+ for (Future<?> future : futures) {
+ future.get();
+ }
+ } catch (Exception e) {
+ throw new RuntimeException(sm.getString("contextConfig.processAnnotationsInParallelFailure"), e);
+ }
+ } finally {
+ if (pool != null) {
+ pool.shutdownNow();
Review comment:
This is not really needed because org.apache.tomcat.util.threads.ScheduledThreadPoolExecutor#shutdown methods do nothing, but anyway it is a good practice to do it.
##########
File path: java/org/apache/catalina/startup/ContextConfig.java
##########
@@ -2136,26 +2146,98 @@ protected InputSource getWebXmlSource(String filename, boolean global) {
}
protected void processAnnotations(Set<WebXml> fragments,
- boolean handlesTypesOnly, Map<String,JavaClassCacheEntry> javaClassCache) {
- for(WebXml fragment : fragments) {
- // Only need to scan for @HandlesTypes matches if any of the
- // following are true:
- // - it has already been determined only @HandlesTypes is required
- // (e.g. main web.xml has metadata-complete="true"
- // - this fragment is for a container JAR (Servlet 3.1 section 8.1)
- // - this fragment has metadata-complete="true"
- boolean htOnly = handlesTypesOnly || !fragment.getWebappJar() ||
- fragment.isMetadataComplete();
-
- WebXml annotations = new WebXml();
- // no impact on distributable
- annotations.setDistributable(true);
- URL url = fragment.getURL();
- processAnnotationsUrl(url, annotations, htOnly, javaClassCache);
- Set<WebXml> set = new HashSet<>();
- set.add(annotations);
- // Merge annotations into fragment - fragment takes priority
- fragment.merge(set);
+ boolean handlesTypesOnly, Map<String, JavaClassCacheEntry> javaClassCache) {
+
+ if (context.getParent() instanceof Host && ((Host) context.getParent()).isParallelAnnotationScanning()) {
+ processAnnotationsInParallel(fragments, handlesTypesOnly, javaClassCache);
+ return;
+ }
+
+ for (WebXml fragment : fragments) {
+ scanWebXmlFragment(handlesTypesOnly, fragment, javaClassCache);
+ }
+ }
+
+ private void scanWebXmlFragment(boolean handlesTypesOnly, WebXml fragment, Map<String, JavaClassCacheEntry> javaClassCache) {
+
+ // Only need to scan for @HandlesTypes matches if any of the
+ // following are true:
+ // - it has already been determined only @HandlesTypes is required
+ // (e.g. main web.xml has metadata-complete="true"
+ // - this fragment is for a container JAR (Servlet 3.1 section 8.1)
+ // - this fragment has metadata-complete="true"
+ boolean htOnly = handlesTypesOnly || !fragment.getWebappJar() ||
+ fragment.isMetadataComplete();
+
+ WebXml annotations = new WebXml();
+ // no impact on distributable
+ annotations.setDistributable(true);
+ URL url = fragment.getURL();
+ processAnnotationsUrl(url, annotations, htOnly, javaClassCache);
+ Set<WebXml> set = new HashSet<>();
+ set.add(annotations);
+ // Merge annotations into fragment - fragment takes priority
+ fragment.merge(set);
+ }
+
+ /**
+ * Executable task to scan a segment for annotations. Each task does the
+ * same work as the for loop inside processAnnotations();
+ *
+ * @author Engebretson, John
+ * @author Kamnani, Jatin
+ */
+ private class AnnotationScanTask implements Callable<Void> {
Review comment:
Why `Callable<Void>` ?
You can use `Runnable`
##########
File path: java/org/apache/catalina/startup/LocalStrings_fr.properties
##########
@@ -69,6 +69,7 @@ contextConfig.missingRealm=Aucun royaume (realm) n'a été configuré pour réal
contextConfig.processAnnotationsDir.debug=Balayage du répertoire pour trouver des fichiers de classe avec annotations [{0}]
contextConfig.processAnnotationsJar.debug=Analyse du fichier jars pour des classes annotées avec [{0}]
contextConfig.processAnnotationsWebDir.debug=Balayage du répertoire d''applications web, pour fichiers de classe avec annotations [{0}]
+contextConfig.processAnnotationsInParallelFailure=exécution parallèle a échoué
Review comment:
Should `exécution` be capital cased like the other messages ?
##########
File path: java/org/apache/catalina/startup/ContextConfig.java
##########
@@ -2136,26 +2146,98 @@ protected InputSource getWebXmlSource(String filename, boolean global) {
}
protected void processAnnotations(Set<WebXml> fragments,
- boolean handlesTypesOnly, Map<String,JavaClassCacheEntry> javaClassCache) {
- for(WebXml fragment : fragments) {
- // Only need to scan for @HandlesTypes matches if any of the
- // following are true:
- // - it has already been determined only @HandlesTypes is required
- // (e.g. main web.xml has metadata-complete="true"
- // - this fragment is for a container JAR (Servlet 3.1 section 8.1)
- // - this fragment has metadata-complete="true"
- boolean htOnly = handlesTypesOnly || !fragment.getWebappJar() ||
- fragment.isMetadataComplete();
-
- WebXml annotations = new WebXml();
- // no impact on distributable
- annotations.setDistributable(true);
- URL url = fragment.getURL();
- processAnnotationsUrl(url, annotations, htOnly, javaClassCache);
- Set<WebXml> set = new HashSet<>();
- set.add(annotations);
- // Merge annotations into fragment - fragment takes priority
- fragment.merge(set);
+ boolean handlesTypesOnly, Map<String, JavaClassCacheEntry> javaClassCache) {
+
+ if (context.getParent() instanceof Host && ((Host) context.getParent()).isParallelAnnotationScanning()) {
+ processAnnotationsInParallel(fragments, handlesTypesOnly, javaClassCache);
+ return;
+ }
+
+ for (WebXml fragment : fragments) {
+ scanWebXmlFragment(handlesTypesOnly, fragment, javaClassCache);
+ }
+ }
+
+ private void scanWebXmlFragment(boolean handlesTypesOnly, WebXml fragment, Map<String, JavaClassCacheEntry> javaClassCache) {
+
+ // Only need to scan for @HandlesTypes matches if any of the
+ // following are true:
+ // - it has already been determined only @HandlesTypes is required
+ // (e.g. main web.xml has metadata-complete="true"
+ // - this fragment is for a container JAR (Servlet 3.1 section 8.1)
+ // - this fragment has metadata-complete="true"
+ boolean htOnly = handlesTypesOnly || !fragment.getWebappJar() ||
+ fragment.isMetadataComplete();
+
+ WebXml annotations = new WebXml();
+ // no impact on distributable
+ annotations.setDistributable(true);
+ URL url = fragment.getURL();
+ processAnnotationsUrl(url, annotations, htOnly, javaClassCache);
+ Set<WebXml> set = new HashSet<>();
+ set.add(annotations);
+ // Merge annotations into fragment - fragment takes priority
+ fragment.merge(set);
+ }
+
+ /**
+ * Executable task to scan a segment for annotations. Each task does the
+ * same work as the for loop inside processAnnotations();
+ *
+ * @author Engebretson, John
+ * @author Kamnani, Jatin
+ */
+ private class AnnotationScanTask implements Callable<Void> {
+ private final WebXml fragment;
+ private final boolean handlesTypesOnly;
+ private Map<String, JavaClassCacheEntry> javaClassCache;
+
+ private AnnotationScanTask(WebXml fragment, boolean handlesTypesOnly, Map<String, JavaClassCacheEntry> javaClassCache) {
+ this.fragment = fragment;
+ this.handlesTypesOnly = handlesTypesOnly;
+ this.javaClassCache = javaClassCache;
+ }
+
+ @Override
+ public Void call() {
+ scanWebXmlFragment(handlesTypesOnly, fragment, javaClassCache);
+
+ return null;
+ }
+
+ }
+
+ /**
+ * Parallelized version of processAnnotationsInParallel(). Constructs tasks,
+ * submits them as they're created, then waits for completion.
+ *
+ * @param fragments Set of parallelizable scans
+ * @param handlesTypesOnly Important parameter for the underlying scan
+ */
+ protected void processAnnotationsInParallel(Set<WebXml> fragments, boolean handlesTypesOnly,
+ Map<String, JavaClassCacheEntry> javaClassCache) {
+
+
+ Server s = getServer();
+ ExecutorService pool = null;
+ try {
+ pool = s.getUtilityExecutor();
+ List<Future<Void>> futures = new ArrayList<>(fragments.size());
+ for (WebXml fragment : fragments) {
+ Callable<Void> task = new AnnotationScanTask(fragment, handlesTypesOnly, javaClassCache);
+ futures.add(pool.submit(task));
+ }
+ try {
+ for (Future<?> future : futures) {
+ future.get();
+ }
+ } catch (Exception e) {
+ throw new RuntimeException(sm.getString("contextConfig.processAnnotationsInParallelFailure"), e);
+ }
+ } finally {
+ if (pool != null) {
+ pool.shutdownNow();
Review comment:
This is not really needed because org.apache.tomcat.util.threads.ScheduledThreadPoolExecutor#shutdown methods do nothing, but anyway it is a good practice to do it.
##########
File path: java/org/apache/catalina/startup/ContextConfig.java
##########
@@ -2136,26 +2146,98 @@ protected InputSource getWebXmlSource(String filename, boolean global) {
}
protected void processAnnotations(Set<WebXml> fragments,
- boolean handlesTypesOnly, Map<String,JavaClassCacheEntry> javaClassCache) {
- for(WebXml fragment : fragments) {
- // Only need to scan for @HandlesTypes matches if any of the
- // following are true:
- // - it has already been determined only @HandlesTypes is required
- // (e.g. main web.xml has metadata-complete="true"
- // - this fragment is for a container JAR (Servlet 3.1 section 8.1)
- // - this fragment has metadata-complete="true"
- boolean htOnly = handlesTypesOnly || !fragment.getWebappJar() ||
- fragment.isMetadataComplete();
-
- WebXml annotations = new WebXml();
- // no impact on distributable
- annotations.setDistributable(true);
- URL url = fragment.getURL();
- processAnnotationsUrl(url, annotations, htOnly, javaClassCache);
- Set<WebXml> set = new HashSet<>();
- set.add(annotations);
- // Merge annotations into fragment - fragment takes priority
- fragment.merge(set);
+ boolean handlesTypesOnly, Map<String, JavaClassCacheEntry> javaClassCache) {
+
+ if (context.getParent() instanceof Host && ((Host) context.getParent()).isParallelAnnotationScanning()) {
+ processAnnotationsInParallel(fragments, handlesTypesOnly, javaClassCache);
+ return;
+ }
+
+ for (WebXml fragment : fragments) {
+ scanWebXmlFragment(handlesTypesOnly, fragment, javaClassCache);
+ }
+ }
+
+ private void scanWebXmlFragment(boolean handlesTypesOnly, WebXml fragment, Map<String, JavaClassCacheEntry> javaClassCache) {
+
+ // Only need to scan for @HandlesTypes matches if any of the
+ // following are true:
+ // - it has already been determined only @HandlesTypes is required
+ // (e.g. main web.xml has metadata-complete="true"
+ // - this fragment is for a container JAR (Servlet 3.1 section 8.1)
+ // - this fragment has metadata-complete="true"
+ boolean htOnly = handlesTypesOnly || !fragment.getWebappJar() ||
+ fragment.isMetadataComplete();
+
+ WebXml annotations = new WebXml();
+ // no impact on distributable
+ annotations.setDistributable(true);
+ URL url = fragment.getURL();
+ processAnnotationsUrl(url, annotations, htOnly, javaClassCache);
+ Set<WebXml> set = new HashSet<>();
+ set.add(annotations);
+ // Merge annotations into fragment - fragment takes priority
+ fragment.merge(set);
+ }
+
+ /**
+ * Executable task to scan a segment for annotations. Each task does the
+ * same work as the for loop inside processAnnotations();
+ *
+ * @author Engebretson, John
+ * @author Kamnani, Jatin
+ */
+ private class AnnotationScanTask implements Callable<Void> {
Review comment:
Why `Callable<Void>` ?
You can use `Runnable`
##########
File path: java/org/apache/catalina/startup/LocalStrings_fr.properties
##########
@@ -69,6 +69,7 @@ contextConfig.missingRealm=Aucun royaume (realm) n'a été configuré pour réal
contextConfig.processAnnotationsDir.debug=Balayage du répertoire pour trouver des fichiers de classe avec annotations [{0}]
contextConfig.processAnnotationsJar.debug=Analyse du fichier jars pour des classes annotées avec [{0}]
contextConfig.processAnnotationsWebDir.debug=Balayage du répertoire d''applications web, pour fichiers de classe avec annotations [{0}]
+contextConfig.processAnnotationsInParallelFailure=exécution parallèle a échoué
Review comment:
Should `exécution` be capital cased like the other messages ?
##########
File path: java/org/apache/catalina/startup/ContextConfig.java
##########
@@ -2136,26 +2146,98 @@ protected InputSource getWebXmlSource(String filename, boolean global) {
}
protected void processAnnotations(Set<WebXml> fragments,
- boolean handlesTypesOnly, Map<String,JavaClassCacheEntry> javaClassCache) {
- for(WebXml fragment : fragments) {
- // Only need to scan for @HandlesTypes matches if any of the
- // following are true:
- // - it has already been determined only @HandlesTypes is required
- // (e.g. main web.xml has metadata-complete="true"
- // - this fragment is for a container JAR (Servlet 3.1 section 8.1)
- // - this fragment has metadata-complete="true"
- boolean htOnly = handlesTypesOnly || !fragment.getWebappJar() ||
- fragment.isMetadataComplete();
-
- WebXml annotations = new WebXml();
- // no impact on distributable
- annotations.setDistributable(true);
- URL url = fragment.getURL();
- processAnnotationsUrl(url, annotations, htOnly, javaClassCache);
- Set<WebXml> set = new HashSet<>();
- set.add(annotations);
- // Merge annotations into fragment - fragment takes priority
- fragment.merge(set);
+ boolean handlesTypesOnly, Map<String, JavaClassCacheEntry> javaClassCache) {
+
+ if (context.getParent() instanceof Host && ((Host) context.getParent()).isParallelAnnotationScanning()) {
+ processAnnotationsInParallel(fragments, handlesTypesOnly, javaClassCache);
+ return;
+ }
+
+ for (WebXml fragment : fragments) {
+ scanWebXmlFragment(handlesTypesOnly, fragment, javaClassCache);
+ }
+ }
+
+ private void scanWebXmlFragment(boolean handlesTypesOnly, WebXml fragment, Map<String, JavaClassCacheEntry> javaClassCache) {
+
+ // Only need to scan for @HandlesTypes matches if any of the
+ // following are true:
+ // - it has already been determined only @HandlesTypes is required
+ // (e.g. main web.xml has metadata-complete="true"
+ // - this fragment is for a container JAR (Servlet 3.1 section 8.1)
+ // - this fragment has metadata-complete="true"
+ boolean htOnly = handlesTypesOnly || !fragment.getWebappJar() ||
+ fragment.isMetadataComplete();
+
+ WebXml annotations = new WebXml();
+ // no impact on distributable
+ annotations.setDistributable(true);
+ URL url = fragment.getURL();
+ processAnnotationsUrl(url, annotations, htOnly, javaClassCache);
+ Set<WebXml> set = new HashSet<>();
+ set.add(annotations);
+ // Merge annotations into fragment - fragment takes priority
+ fragment.merge(set);
+ }
+
+ /**
+ * Executable task to scan a segment for annotations. Each task does the
+ * same work as the for loop inside processAnnotations();
+ *
+ * @author Engebretson, John
+ * @author Kamnani, Jatin
+ */
+ private class AnnotationScanTask implements Callable<Void> {
+ private final WebXml fragment;
+ private final boolean handlesTypesOnly;
+ private Map<String, JavaClassCacheEntry> javaClassCache;
+
+ private AnnotationScanTask(WebXml fragment, boolean handlesTypesOnly, Map<String, JavaClassCacheEntry> javaClassCache) {
+ this.fragment = fragment;
+ this.handlesTypesOnly = handlesTypesOnly;
+ this.javaClassCache = javaClassCache;
+ }
+
+ @Override
+ public Void call() {
+ scanWebXmlFragment(handlesTypesOnly, fragment, javaClassCache);
+
+ return null;
+ }
+
+ }
+
+ /**
+ * Parallelized version of processAnnotationsInParallel(). Constructs tasks,
+ * submits them as they're created, then waits for completion.
+ *
+ * @param fragments Set of parallelizable scans
+ * @param handlesTypesOnly Important parameter for the underlying scan
+ */
+ protected void processAnnotationsInParallel(Set<WebXml> fragments, boolean handlesTypesOnly,
+ Map<String, JavaClassCacheEntry> javaClassCache) {
+
+
+ Server s = getServer();
+ ExecutorService pool = null;
+ try {
+ pool = s.getUtilityExecutor();
+ List<Future<Void>> futures = new ArrayList<>(fragments.size());
+ for (WebXml fragment : fragments) {
+ Callable<Void> task = new AnnotationScanTask(fragment, handlesTypesOnly, javaClassCache);
+ futures.add(pool.submit(task));
+ }
+ try {
+ for (Future<?> future : futures) {
+ future.get();
+ }
+ } catch (Exception e) {
+ throw new RuntimeException(sm.getString("contextConfig.processAnnotationsInParallelFailure"), e);
+ }
+ } finally {
+ if (pool != null) {
+ pool.shutdownNow();
Review comment:
This is not really needed because org.apache.tomcat.util.threads.ScheduledThreadPoolExecutor#shutdown methods do nothing, but anyway it is a good practice to do it.
##########
File path: java/org/apache/catalina/startup/ContextConfig.java
##########
@@ -2136,26 +2146,98 @@ protected InputSource getWebXmlSource(String filename, boolean global) {
}
protected void processAnnotations(Set<WebXml> fragments,
- boolean handlesTypesOnly, Map<String,JavaClassCacheEntry> javaClassCache) {
- for(WebXml fragment : fragments) {
- // Only need to scan for @HandlesTypes matches if any of the
- // following are true:
- // - it has already been determined only @HandlesTypes is required
- // (e.g. main web.xml has metadata-complete="true"
- // - this fragment is for a container JAR (Servlet 3.1 section 8.1)
- // - this fragment has metadata-complete="true"
- boolean htOnly = handlesTypesOnly || !fragment.getWebappJar() ||
- fragment.isMetadataComplete();
-
- WebXml annotations = new WebXml();
- // no impact on distributable
- annotations.setDistributable(true);
- URL url = fragment.getURL();
- processAnnotationsUrl(url, annotations, htOnly, javaClassCache);
- Set<WebXml> set = new HashSet<>();
- set.add(annotations);
- // Merge annotations into fragment - fragment takes priority
- fragment.merge(set);
+ boolean handlesTypesOnly, Map<String, JavaClassCacheEntry> javaClassCache) {
+
+ if (context.getParent() instanceof Host && ((Host) context.getParent()).isParallelAnnotationScanning()) {
+ processAnnotationsInParallel(fragments, handlesTypesOnly, javaClassCache);
+ return;
+ }
+
+ for (WebXml fragment : fragments) {
+ scanWebXmlFragment(handlesTypesOnly, fragment, javaClassCache);
+ }
+ }
+
+ private void scanWebXmlFragment(boolean handlesTypesOnly, WebXml fragment, Map<String, JavaClassCacheEntry> javaClassCache) {
+
+ // Only need to scan for @HandlesTypes matches if any of the
+ // following are true:
+ // - it has already been determined only @HandlesTypes is required
+ // (e.g. main web.xml has metadata-complete="true"
+ // - this fragment is for a container JAR (Servlet 3.1 section 8.1)
+ // - this fragment has metadata-complete="true"
+ boolean htOnly = handlesTypesOnly || !fragment.getWebappJar() ||
+ fragment.isMetadataComplete();
+
+ WebXml annotations = new WebXml();
+ // no impact on distributable
+ annotations.setDistributable(true);
+ URL url = fragment.getURL();
+ processAnnotationsUrl(url, annotations, htOnly, javaClassCache);
+ Set<WebXml> set = new HashSet<>();
+ set.add(annotations);
+ // Merge annotations into fragment - fragment takes priority
+ fragment.merge(set);
+ }
+
+ /**
+ * Executable task to scan a segment for annotations. Each task does the
+ * same work as the for loop inside processAnnotations();
+ *
+ * @author Engebretson, John
+ * @author Kamnani, Jatin
+ */
+ private class AnnotationScanTask implements Callable<Void> {
Review comment:
Why `Callable<Void>` ?
You can use `Runnable`
##########
File path: java/org/apache/catalina/startup/LocalStrings_fr.properties
##########
@@ -69,6 +69,7 @@ contextConfig.missingRealm=Aucun royaume (realm) n'a été configuré pour réal
contextConfig.processAnnotationsDir.debug=Balayage du répertoire pour trouver des fichiers de classe avec annotations [{0}]
contextConfig.processAnnotationsJar.debug=Analyse du fichier jars pour des classes annotées avec [{0}]
contextConfig.processAnnotationsWebDir.debug=Balayage du répertoire d''applications web, pour fichiers de classe avec annotations [{0}]
+contextConfig.processAnnotationsInParallelFailure=exécution parallèle a échoué
Review comment:
Should `exécution` be capital cased like the other messages ?
##########
File path: java/org/apache/catalina/startup/ContextConfig.java
##########
@@ -2136,26 +2146,98 @@ protected InputSource getWebXmlSource(String filename, boolean global) {
}
protected void processAnnotations(Set<WebXml> fragments,
- boolean handlesTypesOnly, Map<String,JavaClassCacheEntry> javaClassCache) {
- for(WebXml fragment : fragments) {
- // Only need to scan for @HandlesTypes matches if any of the
- // following are true:
- // - it has already been determined only @HandlesTypes is required
- // (e.g. main web.xml has metadata-complete="true"
- // - this fragment is for a container JAR (Servlet 3.1 section 8.1)
- // - this fragment has metadata-complete="true"
- boolean htOnly = handlesTypesOnly || !fragment.getWebappJar() ||
- fragment.isMetadataComplete();
-
- WebXml annotations = new WebXml();
- // no impact on distributable
- annotations.setDistributable(true);
- URL url = fragment.getURL();
- processAnnotationsUrl(url, annotations, htOnly, javaClassCache);
- Set<WebXml> set = new HashSet<>();
- set.add(annotations);
- // Merge annotations into fragment - fragment takes priority
- fragment.merge(set);
+ boolean handlesTypesOnly, Map<String, JavaClassCacheEntry> javaClassCache) {
+
+ if (context.getParent() instanceof Host && ((Host) context.getParent()).isParallelAnnotationScanning()) {
+ processAnnotationsInParallel(fragments, handlesTypesOnly, javaClassCache);
+ return;
+ }
+
+ for (WebXml fragment : fragments) {
+ scanWebXmlFragment(handlesTypesOnly, fragment, javaClassCache);
+ }
+ }
+
+ private void scanWebXmlFragment(boolean handlesTypesOnly, WebXml fragment, Map<String, JavaClassCacheEntry> javaClassCache) {
+
+ // Only need to scan for @HandlesTypes matches if any of the
+ // following are true:
+ // - it has already been determined only @HandlesTypes is required
+ // (e.g. main web.xml has metadata-complete="true"
+ // - this fragment is for a container JAR (Servlet 3.1 section 8.1)
+ // - this fragment has metadata-complete="true"
+ boolean htOnly = handlesTypesOnly || !fragment.getWebappJar() ||
+ fragment.isMetadataComplete();
+
+ WebXml annotations = new WebXml();
+ // no impact on distributable
+ annotations.setDistributable(true);
+ URL url = fragment.getURL();
+ processAnnotationsUrl(url, annotations, htOnly, javaClassCache);
+ Set<WebXml> set = new HashSet<>();
+ set.add(annotations);
+ // Merge annotations into fragment - fragment takes priority
+ fragment.merge(set);
+ }
+
+ /**
+ * Executable task to scan a segment for annotations. Each task does the
+ * same work as the for loop inside processAnnotations();
+ *
+ * @author Engebretson, John
+ * @author Kamnani, Jatin
+ */
+ private class AnnotationScanTask implements Callable<Void> {
+ private final WebXml fragment;
+ private final boolean handlesTypesOnly;
+ private Map<String, JavaClassCacheEntry> javaClassCache;
+
+ private AnnotationScanTask(WebXml fragment, boolean handlesTypesOnly, Map<String, JavaClassCacheEntry> javaClassCache) {
+ this.fragment = fragment;
+ this.handlesTypesOnly = handlesTypesOnly;
+ this.javaClassCache = javaClassCache;
+ }
+
+ @Override
+ public Void call() {
+ scanWebXmlFragment(handlesTypesOnly, fragment, javaClassCache);
+
+ return null;
+ }
+
+ }
+
+ /**
+ * Parallelized version of processAnnotationsInParallel(). Constructs tasks,
+ * submits them as they're created, then waits for completion.
+ *
+ * @param fragments Set of parallelizable scans
+ * @param handlesTypesOnly Important parameter for the underlying scan
+ */
+ protected void processAnnotationsInParallel(Set<WebXml> fragments, boolean handlesTypesOnly,
+ Map<String, JavaClassCacheEntry> javaClassCache) {
+
+
+ Server s = getServer();
+ ExecutorService pool = null;
+ try {
+ pool = s.getUtilityExecutor();
+ List<Future<Void>> futures = new ArrayList<>(fragments.size());
+ for (WebXml fragment : fragments) {
+ Callable<Void> task = new AnnotationScanTask(fragment, handlesTypesOnly, javaClassCache);
+ futures.add(pool.submit(task));
+ }
+ try {
+ for (Future<?> future : futures) {
+ future.get();
+ }
+ } catch (Exception e) {
+ throw new RuntimeException(sm.getString("contextConfig.processAnnotationsInParallelFailure"), e);
+ }
+ } finally {
+ if (pool != null) {
+ pool.shutdownNow();
Review comment:
This is not really needed because org.apache.tomcat.util.threads.ScheduledThreadPoolExecutor#shutdown methods do nothing, but anyway it is a good practice to do it.
##########
File path: java/org/apache/catalina/startup/ContextConfig.java
##########
@@ -2136,26 +2146,98 @@ protected InputSource getWebXmlSource(String filename, boolean global) {
}
protected void processAnnotations(Set<WebXml> fragments,
- boolean handlesTypesOnly, Map<String,JavaClassCacheEntry> javaClassCache) {
- for(WebXml fragment : fragments) {
- // Only need to scan for @HandlesTypes matches if any of the
- // following are true:
- // - it has already been determined only @HandlesTypes is required
- // (e.g. main web.xml has metadata-complete="true"
- // - this fragment is for a container JAR (Servlet 3.1 section 8.1)
- // - this fragment has metadata-complete="true"
- boolean htOnly = handlesTypesOnly || !fragment.getWebappJar() ||
- fragment.isMetadataComplete();
-
- WebXml annotations = new WebXml();
- // no impact on distributable
- annotations.setDistributable(true);
- URL url = fragment.getURL();
- processAnnotationsUrl(url, annotations, htOnly, javaClassCache);
- Set<WebXml> set = new HashSet<>();
- set.add(annotations);
- // Merge annotations into fragment - fragment takes priority
- fragment.merge(set);
+ boolean handlesTypesOnly, Map<String, JavaClassCacheEntry> javaClassCache) {
+
+ if (context.getParent() instanceof Host && ((Host) context.getParent()).isParallelAnnotationScanning()) {
+ processAnnotationsInParallel(fragments, handlesTypesOnly, javaClassCache);
+ return;
+ }
+
+ for (WebXml fragment : fragments) {
+ scanWebXmlFragment(handlesTypesOnly, fragment, javaClassCache);
+ }
+ }
+
+ private void scanWebXmlFragment(boolean handlesTypesOnly, WebXml fragment, Map<String, JavaClassCacheEntry> javaClassCache) {
+
+ // Only need to scan for @HandlesTypes matches if any of the
+ // following are true:
+ // - it has already been determined only @HandlesTypes is required
+ // (e.g. main web.xml has metadata-complete="true"
+ // - this fragment is for a container JAR (Servlet 3.1 section 8.1)
+ // - this fragment has metadata-complete="true"
+ boolean htOnly = handlesTypesOnly || !fragment.getWebappJar() ||
+ fragment.isMetadataComplete();
+
+ WebXml annotations = new WebXml();
+ // no impact on distributable
+ annotations.setDistributable(true);
+ URL url = fragment.getURL();
+ processAnnotationsUrl(url, annotations, htOnly, javaClassCache);
+ Set<WebXml> set = new HashSet<>();
+ set.add(annotations);
+ // Merge annotations into fragment - fragment takes priority
+ fragment.merge(set);
+ }
+
+ /**
+ * Executable task to scan a segment for annotations. Each task does the
+ * same work as the for loop inside processAnnotations();
+ *
+ * @author Engebretson, John
+ * @author Kamnani, Jatin
+ */
+ private class AnnotationScanTask implements Callable<Void> {
Review comment:
Why `Callable<Void>` ?
You can use `Runnable`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] kamnani edited a comment on pull request #354: Optimize Server startup time using multi-threading for annotation scanning
Posted by GitBox <gi...@apache.org>.
kamnani edited a comment on pull request #354:
URL: https://github.com/apache/tomcat/pull/354#issuecomment-689023592
@rmaucher I actually tested the optimization with the flag in a large application having 1000's of Jar in the classpath and the server startup gains are significant and the stats are as follows :
2816 jars - 44% improvement
1800 jars - 40% improvement
1000 jars - 40% improvement
The default tomcat app shows an improvement of 6% with the flag.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] kamnani commented on pull request #354: Optimize Server startup time using multi-threading for annotation scanning
Posted by GitBox <gi...@apache.org>.
kamnani commented on pull request #354:
URL: https://github.com/apache/tomcat/pull/354#issuecomment-694934337
Thank you @rmaucher @martin-g to get this moving fast 👍 .
Will this be backported to version 9 ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] martin-g commented on a change in pull request #354: Optimize Server startup time using multi-threading for annotation scanning
Posted by GitBox <gi...@apache.org>.
martin-g commented on a change in pull request #354:
URL: https://github.com/apache/tomcat/pull/354#discussion_r486875285
##########
File path: java/org/apache/catalina/startup/ContextConfig.java
##########
@@ -2136,26 +2146,98 @@ protected InputSource getWebXmlSource(String filename, boolean global) {
}
protected void processAnnotations(Set<WebXml> fragments,
- boolean handlesTypesOnly, Map<String,JavaClassCacheEntry> javaClassCache) {
- for(WebXml fragment : fragments) {
- // Only need to scan for @HandlesTypes matches if any of the
- // following are true:
- // - it has already been determined only @HandlesTypes is required
- // (e.g. main web.xml has metadata-complete="true"
- // - this fragment is for a container JAR (Servlet 3.1 section 8.1)
- // - this fragment has metadata-complete="true"
- boolean htOnly = handlesTypesOnly || !fragment.getWebappJar() ||
- fragment.isMetadataComplete();
-
- WebXml annotations = new WebXml();
- // no impact on distributable
- annotations.setDistributable(true);
- URL url = fragment.getURL();
- processAnnotationsUrl(url, annotations, htOnly, javaClassCache);
- Set<WebXml> set = new HashSet<>();
- set.add(annotations);
- // Merge annotations into fragment - fragment takes priority
- fragment.merge(set);
+ boolean handlesTypesOnly, Map<String, JavaClassCacheEntry> javaClassCache) {
+
+ if (context.getParent() instanceof Host && ((Host) context.getParent()).isParallelAnnotationScanning()) {
+ processAnnotationsInParallel(fragments, handlesTypesOnly, javaClassCache);
+ return;
+ }
+
+ for (WebXml fragment : fragments) {
+ scanWebXmlFragment(handlesTypesOnly, fragment, javaClassCache);
+ }
+ }
+
+ private void scanWebXmlFragment(boolean handlesTypesOnly, WebXml fragment, Map<String, JavaClassCacheEntry> javaClassCache) {
+
+ // Only need to scan for @HandlesTypes matches if any of the
+ // following are true:
+ // - it has already been determined only @HandlesTypes is required
+ // (e.g. main web.xml has metadata-complete="true"
+ // - this fragment is for a container JAR (Servlet 3.1 section 8.1)
+ // - this fragment has metadata-complete="true"
+ boolean htOnly = handlesTypesOnly || !fragment.getWebappJar() ||
+ fragment.isMetadataComplete();
+
+ WebXml annotations = new WebXml();
+ // no impact on distributable
+ annotations.setDistributable(true);
+ URL url = fragment.getURL();
+ processAnnotationsUrl(url, annotations, htOnly, javaClassCache);
+ Set<WebXml> set = new HashSet<>();
+ set.add(annotations);
+ // Merge annotations into fragment - fragment takes priority
+ fragment.merge(set);
+ }
+
+ /**
+ * Executable task to scan a segment for annotations. Each task does the
+ * same work as the for loop inside processAnnotations();
+ *
+ * @author Engebretson, John
+ * @author Kamnani, Jatin
+ */
+ private class AnnotationScanTask implements Callable<Void> {
Review comment:
Why `Callable<Void>` ?
You can use `Runnable`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] rmaucher commented on pull request #354: Optimize Server startup time using multi-threading for annotation scanning
Posted by GitBox <gi...@apache.org>.
rmaucher commented on pull request #354:
URL: https://github.com/apache/tomcat/pull/354#issuecomment-694771979
I merged the PR with some changes and a minor fix. The flag was moved to the Context since that's where it belongs, and it avoids the unnecessary API changes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] kamnani commented on pull request #354: Optimize Server startup time using multi-threading for annotation scanning
Posted by GitBox <gi...@apache.org>.
kamnani commented on pull request #354:
URL: https://github.com/apache/tomcat/pull/354#issuecomment-689023592
@rmaucher I actually tested the optimization with the flag in a large application having 1000's of Jar in the classpath and the server startup gains are significant and the stats are as follows :
2816 classes - 44% improvement
1800 classes - 40% improvement
1000 classes - 40% improvement
The default tomcat app shows an improvement of 6% with the flag.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] kamnani commented on a change in pull request #354: Optimize Server startup time using multi-threading for annotation scanning
Posted by GitBox <gi...@apache.org>.
kamnani commented on a change in pull request #354:
URL: https://github.com/apache/tomcat/pull/354#discussion_r489070249
##########
File path: java/org/apache/catalina/startup/ContextConfig.java
##########
@@ -2136,26 +2146,98 @@ protected InputSource getWebXmlSource(String filename, boolean global) {
}
protected void processAnnotations(Set<WebXml> fragments,
- boolean handlesTypesOnly, Map<String,JavaClassCacheEntry> javaClassCache) {
- for(WebXml fragment : fragments) {
- // Only need to scan for @HandlesTypes matches if any of the
- // following are true:
- // - it has already been determined only @HandlesTypes is required
- // (e.g. main web.xml has metadata-complete="true"
- // - this fragment is for a container JAR (Servlet 3.1 section 8.1)
- // - this fragment has metadata-complete="true"
- boolean htOnly = handlesTypesOnly || !fragment.getWebappJar() ||
- fragment.isMetadataComplete();
-
- WebXml annotations = new WebXml();
- // no impact on distributable
- annotations.setDistributable(true);
- URL url = fragment.getURL();
- processAnnotationsUrl(url, annotations, htOnly, javaClassCache);
- Set<WebXml> set = new HashSet<>();
- set.add(annotations);
- // Merge annotations into fragment - fragment takes priority
- fragment.merge(set);
+ boolean handlesTypesOnly, Map<String, JavaClassCacheEntry> javaClassCache) {
+
+ if (context.getParent() instanceof Host && ((Host) context.getParent()).isParallelAnnotationScanning()) {
+ processAnnotationsInParallel(fragments, handlesTypesOnly, javaClassCache);
+ return;
+ }
+
+ for (WebXml fragment : fragments) {
+ scanWebXmlFragment(handlesTypesOnly, fragment, javaClassCache);
+ }
+ }
+
+ private void scanWebXmlFragment(boolean handlesTypesOnly, WebXml fragment, Map<String, JavaClassCacheEntry> javaClassCache) {
+
+ // Only need to scan for @HandlesTypes matches if any of the
+ // following are true:
+ // - it has already been determined only @HandlesTypes is required
+ // (e.g. main web.xml has metadata-complete="true"
+ // - this fragment is for a container JAR (Servlet 3.1 section 8.1)
+ // - this fragment has metadata-complete="true"
+ boolean htOnly = handlesTypesOnly || !fragment.getWebappJar() ||
+ fragment.isMetadataComplete();
+
+ WebXml annotations = new WebXml();
+ // no impact on distributable
+ annotations.setDistributable(true);
+ URL url = fragment.getURL();
+ processAnnotationsUrl(url, annotations, htOnly, javaClassCache);
+ Set<WebXml> set = new HashSet<>();
+ set.add(annotations);
+ // Merge annotations into fragment - fragment takes priority
+ fragment.merge(set);
+ }
+
+ /**
+ * Executable task to scan a segment for annotations. Each task does the
+ * same work as the for loop inside processAnnotations();
+ *
+ * @author Engebretson, John
+ * @author Kamnani, Jatin
+ */
+ private class AnnotationScanTask implements Callable<Void> {
Review comment:
Thanks for the suggestion.
Resolved with the latest commit.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] rmaucher commented on a change in pull request #354: Optimize Server startup time using multi-threading for annotation scanning
Posted by GitBox <gi...@apache.org>.
rmaucher commented on a change in pull request #354:
URL: https://github.com/apache/tomcat/pull/354#discussion_r485121593
##########
File path: java/org/apache/catalina/mbeans/MBeanFactory.java
##########
@@ -490,6 +491,7 @@ public String createStandardContext(String parent,
public String createStandardHost(String parent, String name,
String appBase,
boolean autoDeploy,
+ boolean parallelAnnotationScanning,
Review comment:
This is not enough to justify changing this API. You can create the host, then set the flag just after. Good enough.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] rmaucher commented on pull request #354: Optimize Server startup time using multi-threading for annotation scanning
Posted by GitBox <gi...@apache.org>.
rmaucher commented on pull request #354:
URL: https://github.com/apache/tomcat/pull/354#issuecomment-693298281
Since the release cycle is done, I'll work on this feature.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] kamnani edited a comment on pull request #354: Optimize Server startup time using multi-threading for annotation scanning
Posted by GitBox <gi...@apache.org>.
kamnani edited a comment on pull request #354:
URL: https://github.com/apache/tomcat/pull/354#issuecomment-687380392
Thanks @rmaucher for your comment. I have made the necessary changes as suggested. It would be great if you could verify that change. I have also updated the PR description since the flags required to enable this have now been changed with this commit.
Is there any other change needed on this PR you want to suggest ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] rmaucher commented on a change in pull request #354: Optimize Server startup time using multi-threading for annotation scanning
Posted by GitBox <gi...@apache.org>.
rmaucher commented on a change in pull request #354:
URL: https://github.com/apache/tomcat/pull/354#discussion_r486924037
##########
File path: java/org/apache/catalina/startup/LocalStrings_fr.properties
##########
@@ -69,6 +69,7 @@ contextConfig.missingRealm=Aucun royaume (realm) n'a été configuré pour réal
contextConfig.processAnnotationsDir.debug=Balayage du répertoire pour trouver des fichiers de classe avec annotations [{0}]
contextConfig.processAnnotationsJar.debug=Analyse du fichier jars pour des classes annotées avec [{0}]
contextConfig.processAnnotationsWebDir.debug=Balayage du répertoire d''applications web, pour fichiers de classe avec annotations [{0}]
+contextConfig.processAnnotationsInParallelFailure=exécution parallèle a échoué
Review comment:
Yes, the translation is meh. As a rule, I'd say never commit any localization IMO, just let others handle in with poeditor later.
##########
File path: java/org/apache/catalina/startup/LocalStrings_fr.properties
##########
@@ -69,6 +69,7 @@ contextConfig.missingRealm=Aucun royaume (realm) n'a été configuré pour réal
contextConfig.processAnnotationsDir.debug=Balayage du répertoire pour trouver des fichiers de classe avec annotations [{0}]
contextConfig.processAnnotationsJar.debug=Analyse du fichier jars pour des classes annotées avec [{0}]
contextConfig.processAnnotationsWebDir.debug=Balayage du répertoire d''applications web, pour fichiers de classe avec annotations [{0}]
+contextConfig.processAnnotationsInParallelFailure=exécution parallèle a échoué
Review comment:
Yes, the translation is meh. As a rule, I'd say never commit any localization IMO, just let others handle in with poeditor later.
##########
File path: java/org/apache/catalina/startup/LocalStrings_fr.properties
##########
@@ -69,6 +69,7 @@ contextConfig.missingRealm=Aucun royaume (realm) n'a été configuré pour réal
contextConfig.processAnnotationsDir.debug=Balayage du répertoire pour trouver des fichiers de classe avec annotations [{0}]
contextConfig.processAnnotationsJar.debug=Analyse du fichier jars pour des classes annotées avec [{0}]
contextConfig.processAnnotationsWebDir.debug=Balayage du répertoire d''applications web, pour fichiers de classe avec annotations [{0}]
+contextConfig.processAnnotationsInParallelFailure=exécution parallèle a échoué
Review comment:
Yes, the translation is meh. As a rule, I'd say never commit any localization IMO, just let others handle in with poeditor later.
##########
File path: java/org/apache/catalina/startup/LocalStrings_fr.properties
##########
@@ -69,6 +69,7 @@ contextConfig.missingRealm=Aucun royaume (realm) n'a été configuré pour réal
contextConfig.processAnnotationsDir.debug=Balayage du répertoire pour trouver des fichiers de classe avec annotations [{0}]
contextConfig.processAnnotationsJar.debug=Analyse du fichier jars pour des classes annotées avec [{0}]
contextConfig.processAnnotationsWebDir.debug=Balayage du répertoire d''applications web, pour fichiers de classe avec annotations [{0}]
+contextConfig.processAnnotationsInParallelFailure=exécution parallèle a échoué
Review comment:
Yes, the translation is meh. As a rule, I'd say never commit any localization IMO, just let others handle in with poeditor later.
##########
File path: java/org/apache/catalina/startup/LocalStrings_fr.properties
##########
@@ -69,6 +69,7 @@ contextConfig.missingRealm=Aucun royaume (realm) n'a été configuré pour réal
contextConfig.processAnnotationsDir.debug=Balayage du répertoire pour trouver des fichiers de classe avec annotations [{0}]
contextConfig.processAnnotationsJar.debug=Analyse du fichier jars pour des classes annotées avec [{0}]
contextConfig.processAnnotationsWebDir.debug=Balayage du répertoire d''applications web, pour fichiers de classe avec annotations [{0}]
+contextConfig.processAnnotationsInParallelFailure=exécution parallèle a échoué
Review comment:
Yes, the translation is meh. As a rule, I'd say never commit any localization IMO, just let others handle in with poeditor later.
##########
File path: java/org/apache/catalina/startup/LocalStrings_fr.properties
##########
@@ -69,6 +69,7 @@ contextConfig.missingRealm=Aucun royaume (realm) n'a été configuré pour réal
contextConfig.processAnnotationsDir.debug=Balayage du répertoire pour trouver des fichiers de classe avec annotations [{0}]
contextConfig.processAnnotationsJar.debug=Analyse du fichier jars pour des classes annotées avec [{0}]
contextConfig.processAnnotationsWebDir.debug=Balayage du répertoire d''applications web, pour fichiers de classe avec annotations [{0}]
+contextConfig.processAnnotationsInParallelFailure=exécution parallèle a échoué
Review comment:
Yes, the translation is meh. As a rule, I'd say never commit any localization IMO, just let others handle in with poeditor later.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] martin-g commented on a change in pull request #354: Optimize Server startup time using multi-threading for annotation scanning
Posted by GitBox <gi...@apache.org>.
martin-g commented on a change in pull request #354:
URL: https://github.com/apache/tomcat/pull/354#discussion_r486870063
##########
File path: java/org/apache/catalina/startup/LocalStrings_fr.properties
##########
@@ -69,6 +69,7 @@ contextConfig.missingRealm=Aucun royaume (realm) n'a été configuré pour réal
contextConfig.processAnnotationsDir.debug=Balayage du répertoire pour trouver des fichiers de classe avec annotations [{0}]
contextConfig.processAnnotationsJar.debug=Analyse du fichier jars pour des classes annotées avec [{0}]
contextConfig.processAnnotationsWebDir.debug=Balayage du répertoire d''applications web, pour fichiers de classe avec annotations [{0}]
+contextConfig.processAnnotationsInParallelFailure=exécution parallèle a échoué
Review comment:
Should `exécution` be capital cased like the other messages ?
##########
File path: java/org/apache/catalina/startup/ContextConfig.java
##########
@@ -2136,26 +2146,98 @@ protected InputSource getWebXmlSource(String filename, boolean global) {
}
protected void processAnnotations(Set<WebXml> fragments,
- boolean handlesTypesOnly, Map<String,JavaClassCacheEntry> javaClassCache) {
- for(WebXml fragment : fragments) {
- // Only need to scan for @HandlesTypes matches if any of the
- // following are true:
- // - it has already been determined only @HandlesTypes is required
- // (e.g. main web.xml has metadata-complete="true"
- // - this fragment is for a container JAR (Servlet 3.1 section 8.1)
- // - this fragment has metadata-complete="true"
- boolean htOnly = handlesTypesOnly || !fragment.getWebappJar() ||
- fragment.isMetadataComplete();
-
- WebXml annotations = new WebXml();
- // no impact on distributable
- annotations.setDistributable(true);
- URL url = fragment.getURL();
- processAnnotationsUrl(url, annotations, htOnly, javaClassCache);
- Set<WebXml> set = new HashSet<>();
- set.add(annotations);
- // Merge annotations into fragment - fragment takes priority
- fragment.merge(set);
+ boolean handlesTypesOnly, Map<String, JavaClassCacheEntry> javaClassCache) {
+
+ if (context.getParent() instanceof Host && ((Host) context.getParent()).isParallelAnnotationScanning()) {
+ processAnnotationsInParallel(fragments, handlesTypesOnly, javaClassCache);
+ return;
+ }
+
+ for (WebXml fragment : fragments) {
+ scanWebXmlFragment(handlesTypesOnly, fragment, javaClassCache);
+ }
+ }
+
+ private void scanWebXmlFragment(boolean handlesTypesOnly, WebXml fragment, Map<String, JavaClassCacheEntry> javaClassCache) {
+
+ // Only need to scan for @HandlesTypes matches if any of the
+ // following are true:
+ // - it has already been determined only @HandlesTypes is required
+ // (e.g. main web.xml has metadata-complete="true"
+ // - this fragment is for a container JAR (Servlet 3.1 section 8.1)
+ // - this fragment has metadata-complete="true"
+ boolean htOnly = handlesTypesOnly || !fragment.getWebappJar() ||
+ fragment.isMetadataComplete();
+
+ WebXml annotations = new WebXml();
+ // no impact on distributable
+ annotations.setDistributable(true);
+ URL url = fragment.getURL();
+ processAnnotationsUrl(url, annotations, htOnly, javaClassCache);
+ Set<WebXml> set = new HashSet<>();
+ set.add(annotations);
+ // Merge annotations into fragment - fragment takes priority
+ fragment.merge(set);
+ }
+
+ /**
+ * Executable task to scan a segment for annotations. Each task does the
+ * same work as the for loop inside processAnnotations();
+ *
+ * @author Engebretson, John
+ * @author Kamnani, Jatin
+ */
+ private class AnnotationScanTask implements Callable<Void> {
+ private final WebXml fragment;
+ private final boolean handlesTypesOnly;
+ private Map<String, JavaClassCacheEntry> javaClassCache;
+
+ private AnnotationScanTask(WebXml fragment, boolean handlesTypesOnly, Map<String, JavaClassCacheEntry> javaClassCache) {
+ this.fragment = fragment;
+ this.handlesTypesOnly = handlesTypesOnly;
+ this.javaClassCache = javaClassCache;
+ }
+
+ @Override
+ public Void call() {
+ scanWebXmlFragment(handlesTypesOnly, fragment, javaClassCache);
+
+ return null;
+ }
+
+ }
+
+ /**
+ * Parallelized version of processAnnotationsInParallel(). Constructs tasks,
+ * submits them as they're created, then waits for completion.
+ *
+ * @param fragments Set of parallelizable scans
+ * @param handlesTypesOnly Important parameter for the underlying scan
+ */
+ protected void processAnnotationsInParallel(Set<WebXml> fragments, boolean handlesTypesOnly,
+ Map<String, JavaClassCacheEntry> javaClassCache) {
+
+
+ Server s = getServer();
+ ExecutorService pool = null;
+ try {
+ pool = s.getUtilityExecutor();
+ List<Future<Void>> futures = new ArrayList<>(fragments.size());
+ for (WebXml fragment : fragments) {
+ Callable<Void> task = new AnnotationScanTask(fragment, handlesTypesOnly, javaClassCache);
+ futures.add(pool.submit(task));
+ }
+ try {
+ for (Future<?> future : futures) {
+ future.get();
+ }
+ } catch (Exception e) {
+ throw new RuntimeException(sm.getString("contextConfig.processAnnotationsInParallelFailure"), e);
+ }
+ } finally {
+ if (pool != null) {
+ pool.shutdownNow();
Review comment:
This is not really needed because org.apache.tomcat.util.threads.ScheduledThreadPoolExecutor#shutdown methods do nothing, but anyway it is a good practice to do it.
##########
File path: java/org/apache/catalina/startup/ContextConfig.java
##########
@@ -2136,26 +2146,98 @@ protected InputSource getWebXmlSource(String filename, boolean global) {
}
protected void processAnnotations(Set<WebXml> fragments,
- boolean handlesTypesOnly, Map<String,JavaClassCacheEntry> javaClassCache) {
- for(WebXml fragment : fragments) {
- // Only need to scan for @HandlesTypes matches if any of the
- // following are true:
- // - it has already been determined only @HandlesTypes is required
- // (e.g. main web.xml has metadata-complete="true"
- // - this fragment is for a container JAR (Servlet 3.1 section 8.1)
- // - this fragment has metadata-complete="true"
- boolean htOnly = handlesTypesOnly || !fragment.getWebappJar() ||
- fragment.isMetadataComplete();
-
- WebXml annotations = new WebXml();
- // no impact on distributable
- annotations.setDistributable(true);
- URL url = fragment.getURL();
- processAnnotationsUrl(url, annotations, htOnly, javaClassCache);
- Set<WebXml> set = new HashSet<>();
- set.add(annotations);
- // Merge annotations into fragment - fragment takes priority
- fragment.merge(set);
+ boolean handlesTypesOnly, Map<String, JavaClassCacheEntry> javaClassCache) {
+
+ if (context.getParent() instanceof Host && ((Host) context.getParent()).isParallelAnnotationScanning()) {
+ processAnnotationsInParallel(fragments, handlesTypesOnly, javaClassCache);
+ return;
+ }
+
+ for (WebXml fragment : fragments) {
+ scanWebXmlFragment(handlesTypesOnly, fragment, javaClassCache);
+ }
+ }
+
+ private void scanWebXmlFragment(boolean handlesTypesOnly, WebXml fragment, Map<String, JavaClassCacheEntry> javaClassCache) {
+
+ // Only need to scan for @HandlesTypes matches if any of the
+ // following are true:
+ // - it has already been determined only @HandlesTypes is required
+ // (e.g. main web.xml has metadata-complete="true"
+ // - this fragment is for a container JAR (Servlet 3.1 section 8.1)
+ // - this fragment has metadata-complete="true"
+ boolean htOnly = handlesTypesOnly || !fragment.getWebappJar() ||
+ fragment.isMetadataComplete();
+
+ WebXml annotations = new WebXml();
+ // no impact on distributable
+ annotations.setDistributable(true);
+ URL url = fragment.getURL();
+ processAnnotationsUrl(url, annotations, htOnly, javaClassCache);
+ Set<WebXml> set = new HashSet<>();
+ set.add(annotations);
+ // Merge annotations into fragment - fragment takes priority
+ fragment.merge(set);
+ }
+
+ /**
+ * Executable task to scan a segment for annotations. Each task does the
+ * same work as the for loop inside processAnnotations();
+ *
+ * @author Engebretson, John
+ * @author Kamnani, Jatin
+ */
+ private class AnnotationScanTask implements Callable<Void> {
Review comment:
Why `Callable<Void>` ?
You can use `Runnable`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org