Github user wangyf2010 commented on the pull request:
https://github.com/apache/tomcat/pull/22#issuecomment-131649270
>My comment regarding separating the refactoring and the functional changes
stands. It is not the refactoring I am against, it is the mixing of refactoring
and functional change in a single commit.
make sense, I can split it into separate commits for refactoring & feature.
>/res/checkstyle
>Not every rule I mentioned is enforced mainly because not all of the
current code follows it (due to the age of the code). We could go through and
make all the changes but that creates a different problem (back-ports get more
difficult). The general rule is a patch should follow the style of the code it
is changing.
Got it, thanks for detail explanation for that.
>Regarding thread safety I can now see that my concern regarding the Java
class cache was mis-placed. However, scanning an individual class triggers the
scanning of all the classes in the hierarchy. If those classes are in different
JARs then you will get the same class scanned by multiple threads. How much of
an issue that is is TBD.
You're right. I can see JavaClassCacheEntry isn't thread safe. Should cover
it.
>While this feature is likely to improve the startup of a Tomcat instance
with a single, large application deployed on a multi-core machine I am far from
convinced it will help when there are multiple applications deployed
particularly when there are more applications deployed than cores available.
That's the feature will benefit single, large application better. Might not
huge benefit for multiple applications. But that's fair enough, we can't rely
on one feature to benefit all cases. And in enterprise environment, one single
web app is deployed in a single tomcat indeed.
>Your lack of concern for the impact this feature may have on running
applications is worring. As currently written it is likely that a deployment
using this feature would have a noticeable performance impact on running
applications if the system was moderately loaded at the time.
It only need resources when scan annotations during server startup, impact
is little. Even without this feature, deploy a new web app onto running tomcat,
it will have performance impact also. What I'm thinking is: mark those forked
maps to be null and let GC recycle them ASAP.
---
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
