Github user markt-asf commented on the pull request:
https://github.com/apache/tomcat/pull/22#issuecomment-130565904
The patch appears to be quite large for a marginal benfit. Separating the
refactoring and the changes into separate commits would enable the true scale
of the changes to be judged.
I remain on the fence about this enhancement, pending a patch that clearly
shows the scale of the changes.
I am -1 on the patch as current written since:
- it is not thread safe (look at the Java class cache)
- it does not follow the coding style of the existing code:
- no tabs (use 4 * spaces for indent)
- no new line before {
- no space after (
- no space before )
- it uses system properties for configuration when they are not necessary.
Configuration for this feature should be at the Context level
- no documentation provided
- no license header in new files
- max coding line length is 100
- max comment line length is 80
- it needs to be dsiabled by default
I also have some more general concerns about this feature:
- Is parallel threads scanning the same class an issue and if so, how do we
prevent that?
- The deployment process is already (or can be) multi-threaded. How much
benefit does this feature provide when multiple apps are being deployed in
parallel?
- What will be the impact on performance for already deployed applications
if this feature is enabled?
- Is a 20s default tieout to scan a JAR appropriate?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
